How was my image upload script exploited

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
dapaintballer331
Forum Newbie
Posts: 2
Joined: Sat Sep 08, 2007 9:54 pm

How was my image upload script exploited

Post by dapaintballer331 »

Somehow users uploaded a file called "muhacir.php" to my image upload folder, and used it to delete EVERYTHING on my website.

How did they get a .jpeg/.gif extension file to upload&rename&execute?
I know mime types can be spoofed, so I thought file extensions would work.

If that isn't the case, how did they upload a .php file? I tried using null bytes on my own server and even that wouldn't work, the filename was still .php%00.jpeg

Any ideas?
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

We will have to see your code.
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

As far as I understood you've tried this:
http://ha.ckers.org/blog/20070604/passi ... imagesize/

Yes, we will need your code :)
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Re: How was my image upload script exploited

Post by superdezign »

dapaintballer331 wrote:I know mime types can be spoofed, so I thought file extensions would work.
You think MIME types are easier to spoof than file extensions?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Re: How was my image upload script exploited

Post by feyd »

superdezign wrote:
dapaintballer331 wrote:I know mime types can be spoofed, so I thought file extensions would work.
You think MIME types are easier to spoof than file extensions?
Both are easy to fake.
dapaintballer331
Forum Newbie
Posts: 2
Joined: Sat Sep 08, 2007 9:54 pm

Post by dapaintballer331 »

feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]

Code: Select all

if(isset($Submit)){
  $file=$_FILES['imagefile']['name'];
  $filetype=substr($file,-4);
  if (!file_exists('upload/'.$_FILES['imagefile']['name'])) {
  if($filetype=="jpeg"|$filetype==".gif"|$filetype==".png"|$filetype==".jpg"|$filetype==".bmp"){
    copy($_FILES['imagefile']['tmp_name'],"upload/".$_FILES['imagefile']['name'])
    or die("Could not copy");
    echo"<br>Upload Complete";
    echo"<br>Name:&nbsp;".$_FILES['imagefile']['name']."";
    echo"<br>Size:&nbsp;".$_FILES['imagefile']['size']." bytes";
	echo"<br>URL: " . $filelocation . "upload/" . $_FILES['imagefile']['name'];
    echo"<br>Type:&nbsp;".$_FILES['imagefile']['type']."<br>";
  }else{
    echo"<br>Upload Error";
    echo"<br>Could Not Copy, Wrong Filetype (".$_FILES['imagefile']['name'].")<br>";
  }
  } else {
  echo 'A image with the same name already exists. Feel free to rename your file then reupload.';
  }
}
Honestly I didn't write the code, but I couldn't have done any better, if I give them direct access to the file like this script does.


feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]
jmut
Forum Regular
Posts: 945
Joined: Tue Jul 05, 2005 3:54 am
Location: Sofia, Bulgaria
Contact:

Post by jmut »

dapaintballer331 wrote:

Code: Select all

if(isset($Submit)){
  $file=$_FILES['imagefile']['name'];
  $filetype=substr($file,-4);
  if (!file_exists('upload/'.$_FILES['imagefile']['name'])) {
  if($filetype=="jpeg"|$filetype==".gif"|$filetype==".png"|$filetype==".jpg"|$filetype==".bmp"){
    copy($_FILES['imagefile']['tmp_name'],"upload/".$_FILES['imagefile']['name'])
    or die("Could not copy");
    echo"<br>Upload Complete";
    echo"<br>Name:&nbsp;".$_FILES['imagefile']['name']."";
    echo"<br>Size:&nbsp;".$_FILES['imagefile']['size']." bytes";
	echo"<br>URL: " . $filelocation . "upload/" . $_FILES['imagefile']['name'];
    echo"<br>Type:&nbsp;".$_FILES['imagefile']['type']."<br>";
  }else{
    echo"<br>Upload Error";
    echo"<br>Could Not Copy, Wrong Filetype (".$_FILES['imagefile']['name'].")<br>";
  }
  } else {
  echo 'A image with the same name already exists. Feel free to rename your file then reupload.';
  }
}
Honestly I didn't write the code, but I couldn't have done any better, if I give them direct access to the file like this script does.

As a start you should not use copy() but rather
http://www.php.net/manual/en/function.m ... d-file.php
mrkite
Forum Contributor
Posts: 104
Joined: Tue Sep 11, 2007 4:19 am

Post by mrkite »

you're using bitwise-or | instead of logical-or ||

This is how they broke your uploader.

Code: Select all

if ($filetype=="jpeg"|$filetype==".gif"|$filetype==".png"|$filetype==".jpg"|$filetype==".bmp")
bitwise-or has a higher precedence than equality. So basically the above is equivalent to:

Code: Select all

if ($filetype==("jpeg"|$filetype)==(".gif"|$filetype)==(".png"|$filetype)==(".jpg"|$filetype)==".bmp")
Change those back to || and try uploading a regular php file


Whenever i have people upload images, I never care what the original filename was.. I use imagemagick to convert their tmp_name into a jpeg before it moves into webspace.

edit: deleted extraneous examples which weren't true for php5.
Post Reply