diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm index 857e4a5d..ae67fac7 100644 --- a/src/lib/Hydra/Controller/Build.pm +++ b/src/lib/Hydra/Controller/Build.pm @@ -170,10 +170,9 @@ sub defaultUriForProduct { sub checkPath { my ($self, $c, $path) = @_; - my $storeDir = $Nix::Config::storeDir . "/"; - error($c, "Invalid path in build product.") - if substr($path, 0, length($storeDir)) ne $storeDir || $path =~ /\/\.\./; - error($c, "Path ‘$path’ is a symbolic link.") if -l $path; + my $path = pathIsInsidePrefix($path, $Nix::Config::storeDir); + error($c, "Build product refers outside of the Nix store.") unless defined $path; + return $path; } @@ -203,7 +202,7 @@ sub download : Chained('build') PathPart { $path .= "/" . join("/", @path) if scalar @path > 0; # Make sure the file is in the Nix store. - checkPath($self, $c, $path); + $path = checkPath($self, $c, $path); # If this is a directory but no "/" is attached, then redirect. if (-d $path && substr($c->request->uri, -1) ne "/") { @@ -247,7 +246,7 @@ sub contents : Chained('build') PathPart Args(1) { my $path = $product->path; - checkPath($self, $c, $path); + $path = checkPath($self, $c, $path); notFound($c, "Product $path has disappeared.") unless -e $path; diff --git a/src/lib/Hydra/Helper/AddBuilds.pm b/src/lib/Hydra/Helper/AddBuilds.pm index aca8fb77..531e1c1f 100644 --- a/src/lib/Hydra/Helper/AddBuilds.pm +++ b/src/lib/Hydra/Helper/AddBuilds.pm @@ -785,15 +785,14 @@ sub addBuildProducts { /^([\w\-]+)\s+([\w\-]+)\s+("[^"]*"|\S+)(\s+(\S+))?$/ or next; my $type = $1; my $subtype = $2 eq "none" ? "" : $2; - my $path = File::Spec->canonpath((substr $3, 0, 1) eq "\"" ? substr $3, 1, -1 : $3); + my $path = substr($3, 0, 1) eq "\"" ? substr($3, 1, -1) : $3; my $defaultPath = $5; # Ensure that the path exists and points into the Nix store. next unless File::Spec->file_name_is_absolute($path); - next if $path =~ /\/\.\./; # don't go up - next unless substr($path, 0, length($storeDir)) eq $storeDir; + $path = pathIsInsidePrefix($path, $Nix::Config::storeDir); + next unless defined $path; next unless -e $path; - next if -l $path; # FIXME: check that the path is in the input closure # of the build? diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm index e93aff4b..b4fcdea0 100644 --- a/src/lib/Hydra/Helper/Nix.pm +++ b/src/lib/Hydra/Helper/Nix.pm @@ -16,7 +16,8 @@ our @EXPORT = qw( getViewResult getLatestSuccessfulViewResult jobsetOverview removeAsciiEscapes getDrvLogPath logContents getMainOutput - getEvals getMachines); + getEvals getMachines + pathIsInsidePrefix); sub getHydraHome { @@ -402,4 +403,45 @@ sub getMachines { } +# Check whether ‘$path’ is inside ‘$prefix’. In particular, it checks +# that resolving symlink components of ‘$path’ never takes us outside +# of ‘$prefix’. We use this to check that Nix build products don't +# refer to things outside of the Nix store (e.g. /etc/passwd) or to +# symlinks outside of the store that point into the store +# (e.g. /run/current-system). Return undef or the resolved path. +sub pathIsInsidePrefix { + my ($path, $prefix) = @_; + my $n = 0; + $path =~ s/\/+/\//g; # remove redundant slashes + $path =~ s/\/*$//; # remove trailing slashes + + return undef unless $path eq $prefix || substr($path, 0, length($prefix) + 1) eq "$prefix/"; + + my @cs = File::Spec->splitdir(substr($path, length($prefix) + 1)); + my $cur = $prefix; + + foreach my $c (@cs) { + next if $c eq "."; + + # ‘..’ should not take us outside of the prefix. + if ($c eq "..") { + return if length($cur) <= length($prefix); + $cur =~ s/\/[^\/]*$// or die; # remove last component + next; + } + + my $new = "$cur/$c"; + if (-l $new) { + my $link = readlink $new or return undef; + $new = substr($link, 0, 1) eq "/" ? $link : "$cur/$link"; + $new = pathIsInsidePrefix($new, $prefix); + return undef unless defined $new; + } + $cur = $new; + } + + return $cur; +} + + 1;