Fix critical bug in symlink command handling found during testing
authorJacob Bachmeyer <jcb@gnu.org>
Sat, 29 Jan 2022 02:56:49 +0000 (20:56 -0600)
committerJacob Bachmeyer <jcb@gnu.org>
Sat, 29 Jan 2022 02:56:49 +0000 (20:56 -0600)
Previously, while symlink targets were checked for the string "..", symlink
names were unchecked; this allowed symlinks to be placed outside of the
permitted areas for which the signing key is authorized and even outside of the
managed file tree, requiring only that the containing directory already exist.
The test case places a symlink directly into the top-level pub/ directory to
demonstrate the issue and confirm that it is fixed.

I consider this bug critical because while the rogue symlink can only refer to
something else at or below its own location, it could replace an existing
symlink.  While I do not expect that this provides any way to crack system
security, careful misuse could certainly cause considerable nuisance, possibly
breaking the entire system if an attacker can find a symlink that is critical
for the system's operation and replace it with a dangling symlink.

upload-ftp-v1.2.pl

index f3d085d81e4e4b6cd39d336f9ad97fa4c23a3e03..0f0f42cdefe53b0f0a33712ec0e557b35acbef56 100755 (executable)
@@ -841,7 +841,7 @@ sub read_directive_file {
     } elsif ($tainted_cmd =~ /^symlink:?$/i) {  # case-insensitive, w or w/o the :
       $tainted_val =~ /^([\w_+][-.\w_+\/]*)\s+([\w_+][-.\w_+\/]*)$/ || &fatal("invalid parameters for symlink command: $tainted_val",1,$directive_file_contents);
       my ($target,$link) = ($1,$2);  # so far so good
-      &fatal("invalid parameters for symlink command(2): $tainted_val",1,$directive_file_contents) if ($target =~ /\.\./);
+      &fatal("invalid parameters for symlink command(2): $tainted_val",1,$directive_file_contents) if ($target =~ /\.\./ || $link =~ /\.\./);
       $info{"symlink-$target"} = {"link" => $link, "order" => $cnt++}; #ok.
     } elsif ($tainted_cmd =~ /^rmsymlink:?$/i) {  # case-insensitive, w or w/o the :
       $tainted_val =~ /^([\w_+][-.\w_+\/]*)$/ || &fatal("invalid parameters for rmsymlink command: $tainted_val",1,$directive_file_contents);