Item15191: fixed upload filter

an uploaded html file is secured by appending txt multiple times html -> html.txt -> html.txt.txt -> ...
foswiki · May 5, 2023 · ef334dd · ef334dd
1 parent d953a86
commit ef334dd
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 9 deletions.
diff --git a/UnitTestContrib/test/unit/RobustnessTests.pm b/UnitTestContrib/test/unit/RobustnessTests.pm
@@ -207,8 +207,8 @@ sub test_sanitizeAttachmentName {
     $Foswiki::cfg{UploadFilter} = qr(^(
          (?i)\.htaccess                       # .htaccess needs to be case insensitive
         | .*\.(?i)                            # Case insensitive
-           (?:php[0-9s]?(\..*)?               # PHP files can have a suffix
-            | [sp]?htm[l]?(\..*)?             # html, shtml, phtml, htm, shtm, phtm
+           (?:php[0-9s]?                      # PHP files can have a suffix
+            | [sp]?htm[l]?                    # html, shtml, phtml, htm, shtm, phtm
             | pl                              # Perl
             | py                              # Python
             | cgi                             # CGI Scripts
@@ -223,16 +223,11 @@ sub test_sanitizeAttachmentName {
     # Trailing dot ignored on windows
     $this->assert_str_equals( ".HTacceSS..txt", _sanitize(".HTacceSS.") );
 
-    for my $i (qw(php shtm phtml  html pl py cgi PHP SHTM PHTML PL PY CGI)) {
+    for my $i (qw(php shtm phtml html pl py cgi PHP SHTM PHTML PL PY CGI)) {
         my $j = "bog.$i";
         my $y = "$j.txt";
         $this->assert_str_equals( $y, _sanitize($j) );
     }
-    for my $i (qw(php php0 phtm shtml html PHP PHP0 PHTM SHTML)) {
-        my $j = "bog.$i.s";
-        my $y = "$j.txt";
-        $this->assert_str_equals( $y, _sanitize($j) );
-    }
 
     # Trailing dots also get renamed
     for my $i (qw(php phtm shtml html PHP py pl cgi PHTM SHTML)) {
@@ -241,6 +236,10 @@ sub test_sanitizeAttachmentName {
         $this->assert_str_equals( $y, _sanitize($j) );
     }
 
+    # Item15191: secured attachemts must not be double secured
+    $this->assert_str_equals( "test.html.txt", _sanitize("test.html.txt") );
+    $this->assert_str_equals( "test.php.txt",  _sanitize("test.php.txt") );
+
     return;
 }
 

diff --git a/core/lib/Foswiki.spec b/core/lib/Foswiki.spec
@@ -1008,7 +1008,7 @@ $Foswiki::cfg{DenyDotDotInclude} = $TRUE;
 # files that were already uploaded, or files that were created directly
 # on the server.
 #
-$Foswiki::cfg{UploadFilter} = '^((?i)\.htaccess|.*\.(?i)(?:php[0-9s]?(\..*)?|[sp]?htm[l]?(\..*)?|pl|py|cgi)?)$';
+$Foswiki::cfg{UploadFilter} = '^((?i)\.htaccess|.*\.(?i)(?:php[0-9s]?|[sp]?htm[l]?|pl|py|cgi)?)$';
 
 # **REGEX LABEL="Name Filter" EXPERT**
 # Filter-out regex for web names, topic names, usernames,