Is this script adequate to prevent malicious uploading of files?

General Web Dev
1

I recently created this Utility Reviews wanted post and used the following script to rename any PHP files to .TXT files.

#  get the absolute path to $file
   $path = pathinfo(realpath($downloadedZip), PATHINFO_DIRNAME);

   $zip = new ZipArchive;
  $res = $zip->open($downloadedZip);
if($res===true):
  for ($i2=0; $i2<$zip->numFiles; $i2++) :
     # MAYBE RENAME
        $BAD = $zip->statIndex($i2)['name'];
        $BAD = strtolower($BAD);
        if( strpos($BAD, '.php') ) : 
           rename('DOWNLOADS/' .$BAD, 'DOWNLOADS/' .$BAD .'-NOT-ALLOWED.txt');
        endif;
   endfor;
endif; //($res===true):

Could there be any problems from JavaScript files, etc?

2

strpos is technically insufficient. Verify that the position returned is the last 4 characters of the filename, because people do weird things like name files myfile.phpeditor.wark and that would get caught by a flat strpos check.

As far as javascript files… any HTML file can carry a javascript payload that can be executed, but it would be a client-side script at that point, not a server-side one.

3

You can add a .htaccess file to prevent php being run in the folder containing the uploaded files.

3 Likes
4

As suggested I created a myfile.phpeditor.wark with PHP content :

  1. tried in three browsers:
    a. Opera displayed PHP content
    b. Chromium displayed PHP content
    c. Firefox displayed a blank screen

It looks as though Apache2 only recognises PHP extensions and prevents non PHP files from running.

The thought did occur that JavaScript maybe could be used to rename a file such as index.html to index.php.. Activating the renamed file could create havoc :frowning:

5

Nono… it wasnt that the PHP content would be executed, it’s that your script would false-positive a non-PHP file with the name myfile.phpeditor.wark

1 Like
6

AHH now I understand :slight_smile:

Perhaps I should ensure the file extension is .php

$file = 'myfile.phpeditor.wark ';
if( '.php' === substr($file, -4) ) :
	echo '<br><br> Not allowed';
	exit;
else:
   echo '<br><br> this $file OK ==> ' .$file;
endif;	


$file = 'index-just-testing.php';
if( '.php' === substr($file, -4) ) :
	echo '<br><br> this $file Not allowed ==> ' .$file ;
	exit;
else:
   echo '<br><br> this $file OK ==> ' .$file;
endif;

Output:

this $file OK ==> myfile.phpeditor.wark

this $file Not allowed ==> index-just-testing.php

1 Like
7

Many thanks.

I tried a couple of examples and this one seems the easiest:

.htaccess

php_flag engine off
8

Testing for an extension string is OK I guess, but I recommend you do a bit more too. i.e.
https://www.php.net/manual/en/function.mime-content-type.php

Example #1 mime_content_type() Example

<?php
echo mime_content_type('php.gif') . "\n";
echo mime_content_type('test.php');
?>

The above example will output:

image/gif
text/plain

1 Like
9

Many thanks for the suggestions.

I updated and completely revamped the site. Full page view is now possible and ideal for mobiles and laptops. Pasting HTML script into the following we-page automatically adds the page to Default.zip. Not even a Zip Name or filename is required :slight_smile:

Upload, Zip and Render HTML Script

Wherever the file has a lowercase .php extension I appended .txt. This seems adequate because I am just concerned about preventing the .php file from being called.

Viewing the content as a .txt file seems hamless and hopefully will not cause any problems.

11

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.