This should fix the image upload bug for good. Uses basename() and upload verificatio...

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 341318adf26a82036906aae52c11b8677706c871..2182ff680228c52224bb7df41488bb79bf752963 100644 (file)
--- 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;
         }
@@ -118,17 +118,18 @@ switch ($serendipity['GET']['adminAction']) {
     // First find out whether to fetch a file or accept an upload
     if ($serendipity['POST']['imageurl'] != '' && $serendipity['POST']['imageurl'] != 'http://') {
         if (!empty($serendipity['POST']['target_filename'])) {
-            $tfile   = serendipityNormalizeFilename($serendipity['POST']['target_filename']);
+            $tfile   = $serendipity['POST']['target_filename'];
         } else {
-            $tfile   = serendipityNormalizeFilename(basename($serendipity['POST']['imageurl']));
+            $tfile   = $serendipity['POST']['imageurl'];
         }
+        
+        $tfile = serendipity_uploadSecure(basename($tfile));
 
-        if ($serendipity['serendipityUserlevel'] < USERLEVEL_ADMIN && serendipity_isActiveFile($tfile)) {
+        if (serendipity_isActiveFile($tfile)) {
             printf(ERROR_FILE_FORBIDDEN, $tfile);
             break;
         }
 
-        $tfile = serendipityNormalizeFilename(serendipity_uploadSecure($tfile));
         $serendipity['POST']['target_directory'] = serendipity_uploadSecure($serendipity['POST']['target_directory'], true, true);
         $target = $serendipity['serendipityPath'] . $serendipity['uploadPath'] . $serendipity['POST']['target_directory'] . $tfile;
 
@@ -171,17 +172,18 @@ switch ($serendipity['GET']['adminAction']) {
         }
     } else {
         if (!empty($serendipity['POST']['target_filename'])) {
-            $tfile   = serendipityNormalizeFilename($serendipity['POST']['target_filename']);
+            $tfile   = $serendipity['POST']['target_filename'];
         } else {
-            $tfile   = serendipityNormalizeFilename($_FILES['userfile']['name']);
+            $tfile   = $_FILES['userfile']['name'];
         }
 
-        if ($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);
             break;
         }
 
-        $tfile = serendipityNormalizeFilename(serendipity_uploadSecure($tfile));
         $serendipity['POST']['target_directory'] = serendipity_uploadSecure($serendipity['POST']['target_directory'], true, true);
         $target = $serendipity['serendipityPath'] . $serendipity['uploadPath'] . $serendipity['POST']['target_directory'] . $tfile;
 
@@ -189,7 +191,7 @@ switch ($serendipity['GET']['adminAction']) {
             echo '(' . $target . ') ' . ERROR_FILE_EXISTS_ALREADY;
         } else {
             // Accept file
-            if (move_uploaded_file($_FILES['userfile']['tmp_name'], $target)) {
+            if (is_uploaded_file($_FILES['userfile']['tmp_name']) && move_uploaded_file($_FILES['userfile']['tmp_name'], $target)) {
                 printf(FILE_UPLOADED . '<br />', $_FILES['userfile']['name'], $target);
                 @umask(0000);
                 @chmod($target, 0664);

diff --git a/include/functions_images.inc.php b/include/functions_images.inc.php
index fdb75065bb3f264c6e42d6b561d8868f40c974a2..d8ca99a58eb2840aef3649b22183fedc5b25704f 100644 (file)
--- 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 .= '/';
         }
     }