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.8.1~4
X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=fe82d382684966dcb1442b96731eb7d03d1aaafe;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 341318a..2182ff6 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;
         }
@@ -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 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 .= '/';
         }
     }