forked from lix-project/hydra
Security: Improve checking of build products
Build product paths cannot reference locations outside of the Nix store. We previously disallowed paths from being symlinks, but this didn't take into account that parent path elements can be symlinks as well. So a build product /nix/store/bla.../foo/passwd, with /nix/store/bla.../foo being a symlink to /etc, would still work. So now we check all paths encountered during path resolution. Symlinks are allowed again so long as they point to the Nix store.
This commit is contained in:
parent
94984270b0
commit
e7926e046b
|
@ -170,10 +170,9 @@ sub defaultUriForProduct {
|
||||||
|
|
||||||
sub checkPath {
|
sub checkPath {
|
||||||
my ($self, $c, $path) = @_;
|
my ($self, $c, $path) = @_;
|
||||||
my $storeDir = $Nix::Config::storeDir . "/";
|
my $path = pathIsInsidePrefix($path, $Nix::Config::storeDir);
|
||||||
error($c, "Invalid path in build product.")
|
error($c, "Build product refers outside of the Nix store.") unless defined $path;
|
||||||
if substr($path, 0, length($storeDir)) ne $storeDir || $path =~ /\/\.\./;
|
return $path;
|
||||||
error($c, "Path ‘$path’ is a symbolic link.") if -l $path;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -203,7 +202,7 @@ sub download : Chained('build') PathPart {
|
||||||
$path .= "/" . join("/", @path) if scalar @path > 0;
|
$path .= "/" . join("/", @path) if scalar @path > 0;
|
||||||
|
|
||||||
# Make sure the file is in the Nix store.
|
# 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 this is a directory but no "/" is attached, then redirect.
|
||||||
if (-d $path && substr($c->request->uri, -1) ne "/") {
|
if (-d $path && substr($c->request->uri, -1) ne "/") {
|
||||||
|
@ -247,7 +246,7 @@ sub contents : Chained('build') PathPart Args(1) {
|
||||||
|
|
||||||
my $path = $product->path;
|
my $path = $product->path;
|
||||||
|
|
||||||
checkPath($self, $c, $path);
|
$path = checkPath($self, $c, $path);
|
||||||
|
|
||||||
notFound($c, "Product $path has disappeared.") unless -e $path;
|
notFound($c, "Product $path has disappeared.") unless -e $path;
|
||||||
|
|
||||||
|
|
|
@ -785,15 +785,14 @@ sub addBuildProducts {
|
||||||
/^([\w\-]+)\s+([\w\-]+)\s+("[^"]*"|\S+)(\s+(\S+))?$/ or next;
|
/^([\w\-]+)\s+([\w\-]+)\s+("[^"]*"|\S+)(\s+(\S+))?$/ or next;
|
||||||
my $type = $1;
|
my $type = $1;
|
||||||
my $subtype = $2 eq "none" ? "" : $2;
|
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;
|
my $defaultPath = $5;
|
||||||
|
|
||||||
# Ensure that the path exists and points into the Nix store.
|
# Ensure that the path exists and points into the Nix store.
|
||||||
next unless File::Spec->file_name_is_absolute($path);
|
next unless File::Spec->file_name_is_absolute($path);
|
||||||
next if $path =~ /\/\.\./; # don't go up
|
$path = pathIsInsidePrefix($path, $Nix::Config::storeDir);
|
||||||
next unless substr($path, 0, length($storeDir)) eq $storeDir;
|
next unless defined $path;
|
||||||
next unless -e $path;
|
next unless -e $path;
|
||||||
next if -l $path;
|
|
||||||
|
|
||||||
# FIXME: check that the path is in the input closure
|
# FIXME: check that the path is in the input closure
|
||||||
# of the build?
|
# of the build?
|
||||||
|
|
|
@ -16,7 +16,8 @@ our @EXPORT = qw(
|
||||||
getViewResult getLatestSuccessfulViewResult
|
getViewResult getLatestSuccessfulViewResult
|
||||||
jobsetOverview removeAsciiEscapes getDrvLogPath logContents
|
jobsetOverview removeAsciiEscapes getDrvLogPath logContents
|
||||||
getMainOutput
|
getMainOutput
|
||||||
getEvals getMachines);
|
getEvals getMachines
|
||||||
|
pathIsInsidePrefix);
|
||||||
|
|
||||||
|
|
||||||
sub getHydraHome {
|
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;
|
1;
|
||||||
|
|
Loading…
Reference in a new issue