From: garvinhicking <garvinhicking>
Date: Fri, 13 May 2005 11:04:42 +0000 (+0000)
Subject: This should fix the image upload bug for good. Uses basename() and upload verificatio... 
X-Git-Tag: 0.9~461
X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=f899929029d33201e866171c68e3470b6474fc21;p=s9y.git

This should fix the image upload bug for good. Uses basename() and upload verification before any checks are done.
Also admins can no longer upload active content files.

Tricking the upload by making the directory "evil.ph" and the filename "p" does not work because trailing slashes are appended to directory names.
---

diff --git a/include/admin/images.inc.php b/include/admin/images.inc.php
index 5903746..36523cd 100644
--- a/include/admin/images.inc.php
+++ b/include/admin/images.inc.php
@@ -60,7 +60,7 @@ switch ($serendipity['GET']['adminAction']) {
             return;
         }
 
-        if ($serendipity['serendipityUserlevel'] < USERLEVEL_ADMIN && serendipity_isActiveFile($serendipity['GET']['newname'])) {
+        if (serendipity_isActiveFile(basename($serendipity['GET']['newname']))) {
             printf(ERROR_FILE_FORBIDDEN, $serendipity['GET']['newname']);
             return;
         }
@@ -119,23 +119,24 @@ switch ($serendipity['GET']['adminAction']) {
     if ($serendipity['POST']['imageurl'] != '' && $serendipity['POST']['imageurl'] != 'http://') {
         if (!empty($serendipity['POST']['target_filename'][2])) {
             // Faked hidden form 2 when submitting with JavaScript
-            $tfile   = serendipityNormalizeFilename($serendipity['POST']['target_filename'][2]);
+            $tfile   = $serendipity['POST']['target_filename'][2];
             $tindex  = 2;
         } elseif (!empty($serendipity['POST']['target_filename'][1])) {
             // Fallback key when not using JavaScript
-            $tfile   = serendipityNormalizeFilename($serendipity['POST']['target_filename'][1]);
+            $tfile   = $serendipity['POST']['target_filename'][1];
             $tindex  = 1;
         } else {
-            $tfile   = serendipityNormalizeFilename(basename($serendipity['POST']['imageurl']));
+            $tfile   = $serendipity['POST']['imageurl'];
             $tindex  = 1;
         }
 
-        if ($serendipity['serendipityUserlevel'] < USERLEVEL_ADMIN && serendipity_isActiveFile($tfile)) {
+        $tfile = serendipity_uploadSecure(basename($tfile));
+
+        if (serendipity_isActiveFile($tfile)) {
             printf(ERROR_FILE_FORBIDDEN, $tfile);
             break;
         }
 
-        $tfile = serendipityNormalizeFilename(serendipity_uploadSecure($tfile));
         $serendipity['POST']['target_directory'][$tindex] = serendipity_uploadSecure($serendipity['POST']['target_directory'][$tindex], true, true);
         $target = $serendipity['serendipityPath'] . $serendipity['uploadPath'] . $serendipity['POST']['target_directory'][$tindex] . $tfile;
 
@@ -185,21 +186,22 @@ switch ($serendipity['GET']['adminAction']) {
             $uploadfile = &$_FILES['serendipity']['name']['userfile'][$idx];
             $uploadtmp  = &$_FILES['serendipity']['tmp_name']['userfile'][$idx];
             if (!empty($target_filename)) {
-                $tfile   = serendipityNormalizeFilename($target_filename);
+                $tfile   = $target_filename;
             } elseif (!empty($uploadfile)) {
-                $tfile   = serendipityNormalizeFilename($uploadfile);
+                $tfile   = $uploadfile;
             } else {
                 // skip empty array
                 continue;
             }
             
-	    if (preg_match('@^\.@', $tfile) || ($serendipity['serendipityUserlevel'] < USERLEVEL_ADMIN && (preg_match('@\.(php[34]?|[ps]html?)$@i', $tfile)))) {
+            $tfile = serendipity_uploadSecure(basename($tfile));
+
+	        if (serendipity_isActiveFile($tfile)) {
                 printf(ERROR_FILE_FORBIDDEN, $tfile);
                 echo '<br />';
                 continue;
             }
     
-            $tfile = serendipityNormalizeFilename(serendipity_uploadSecure($tfile));
             $serendipity['POST']['target_directory'][$idx] = serendipity_uploadSecure($serendipity['POST']['target_directory'][$idx], true, true);
             $target = $serendipity['serendipityPath'] . $serendipity['uploadPath'] . $serendipity['POST']['target_directory'][$idx] . $tfile;
     
@@ -208,7 +210,7 @@ switch ($serendipity['GET']['adminAction']) {
                 echo '<br />';
             } else {
                 // Accept file
-                if (move_uploaded_file($uploadtmp, $target)) {
+                if (is_uploaded_file($uploadtmp) && move_uploaded_file($uploadtmp, $target)) {
                     printf(FILE_UPLOADED . '<br />', $uploadfile, $target);
                     @umask(0000);
                     @chmod($target, 0664);
diff --git a/include/functions_images.inc.php b/include/functions_images.inc.php
index fdb7506..d8ca99a 100644
--- a/include/functions_images.inc.php
+++ b/include/functions_images.inc.php
@@ -2,14 +2,6 @@
 # Copyright (c) 2003-2005, Jannis Hermanns (on behalf the Serendipity Developer Team)
 # All rights reserved.  See LICENSE file for licensing details
 
-/**
-* Normalize a filename
-**/
-function serendipityNormalizeFilename($in) {
-    $out = preg_replace('![^a-zA-Z0-9\._/-]!', '', $in);
-    return $out;
-}
-
 function serendipity_isActiveFile($file) {
     if (preg_match('@^\.@', $file)) {
         return true;
@@ -1252,7 +1244,7 @@ function serendipity_uploadSecure($var, $strip_paths = true, $append_slash = fal
     $var = preg_replace('@^(/+)@', '', $var);
     
     if ($append_slash) {
-        if (substr($var, -1, 1) != '/') {
+        if (!empty($var) && substr($var, -1, 1) != '/') {
             $var .= '/';
         }
     }