Page 1 of 1

Checking for spaces in filename when uploading a file

Posted: Tue Apr 29, 2008 7:19 am
by deejay
~pickle | Please use [ code=html ], [ code=php ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.


What I have is a page where people can upload photos of their properties -

what I need to do is check for any spaces in filename if there are any.

here is the code I have that handles the image values sent over from the form.

Code: Select all

 
 
if ($HTTP_POST_FILES['img1']['name']['size']>$max_size) 
{ 
echo "The file for image 1 is too big - Please go back to the Homeowner Admin and edit your listing <a href=\"index.php\" > Click Here.</a><br>\n"; exit;
 
 }
else 
{ 
$imgone=$HTTP_POST_FILES['img1']['name'];
}
 
 
iknow that what if it was a straight forward check on a filename I could use

Code: Select all

 
 
$imgone = str_replace ( ' ', '_', $imgone );
 
 


if i could get the file uploading to have a '_' instead of space and the filename stored in mysql the same my problem would be solved.

can anyone point me in the right direction on how to fit this into my code


~pickle | Please use [ code=html ], [ code=php ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.

Re: Checking for spaces in filename when uploading a file

Posted: Tue Apr 29, 2008 8:31 am
by mamad876
I think you can simply move uploaded file with

Code: Select all

bool move_uploaded_file  ( string $filename  , string $destination  )

PHP function, and then rename it with

Code: Select all

bool rename  ( string $oldname  , string $newname  [, resource $context  ] )
PHP function, and store the new file name in database.

or you can open the file in tmp upload folder and then copy it's content to new file that you created with whatever name you want.

But I think the first solution it much simpler and safer.

Regards,
MAMAD

Re: Checking for spaces in filename when uploading a file

Posted: Tue Apr 29, 2008 9:31 am
by Mordred
mamad876 wrote:I think you can simply move uploaded file with

Code: Select all

bool move_uploaded_file  ( string $filename  , string $destination  )

PHP function, and then rename it with

Code: Select all

bool rename  ( string $oldname  , string $newname  [, resource $context  ] )
Duh.

Code: Select all

bool move_uploaded_file  ( string $filename  , string [b]$newname[/b]  )

Re: Checking for spaces in filename when uploading a file

Posted: Tue Apr 29, 2008 9:58 am
by pickle
$HTTP_POST_FILES is deprecated, you should be using $_FILES.

Re: Checking for spaces in filename when uploading a file

Posted: Tue Apr 29, 2008 10:13 am
by Mordred
Ah, I also forgot the most important thing (besides what pickle already said)

Don't use the user-supplied name for writing the file. Generate your own random name (you can keep the original name in the database if you think it matters).

Re: Checking for spaces in filename when uploading a file

Posted: Thu May 01, 2008 3:45 am
by deejay
~pickle | Please use [ code=html ], [ code=php ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.


thanks for that - helped me get my head around it.

Code: Select all

 
 
$img = str_replace ( ' ', '_', $img );  
 
$res = move_uploaded_file($_FILES['img']['tmp_name'], $fullpath .
$img);
 
 
how simple, can't believe I couldn't my head around that one.


Cheers


~pickle | Please use [ code=html ], [ code=php ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.

Re: Checking for spaces in filename when uploading a file

Posted: Thu May 01, 2008 3:50 am
by onion2k
Mordred wrote:Don't use the user-supplied name for writing the file. Generate your own random name (you can keep the original name in the database if you think it matters).
If you sanitise the name to remove anything that isn't alphanumeric, what's wrong with using it? Random asset names in a site are horrible ... I'm much rather have pictureofadog.jpg than iojefsdwq.jpg.

Re: Checking for spaces in filename when uploading a file

Posted: Thu May 01, 2008 12:45 pm
by Mordred
onion2k wrote:
Mordred wrote:Don't use the user-supplied name for writing the file. Generate your own random name (you can keep the original name in the database if you think it matters).
If you sanitise the name to remove anything that isn't alphanumeric, what's wrong with using it? Random asset names in a site are horrible ... I'm much rather have pictureofadog.jpg than iojefsdwq.jpg.
If it's now what the user wanted, does it matter how you changed it? Who cares how the assets are called, as long as they are accessible? And btw, pictureofadog.php is alphanumeric as well.

But that's just counter arguments to what you said. The real reason I recommend random names (which are out of the web root and accessed through a proxy script with no indication of the real FS name) is local file includes. True, not much of a threat for experienced coders, but with newbies - who knows (I do; it's common enough). The thing is - if you're going to change the name anyway, better do it in a way so you can protect yourself from a wider attack tree. With one simple step you shoot the entire attack tree.

Re: Checking for spaces in filename when uploading a file

Posted: Thu May 01, 2008 2:19 pm
by onion2k
Mordred wrote:Who cares how the assets are called, as long as they are accessible?
Google does. Filenames (well, urls) are a factor in your page rank when you're getting things into Google Image Search. As for a proxy script.. they work well enough, but Apache accessing the asset directly is a lot faster, especially if you're going to a database to find the asset name.

The point here is that you're alluding to there being a security risk of some description if you use the original filename. I really don't see what risk there is so long as you remove anything that isn't alphanumeric. How is a stored image that a user has uploaded a potential vector for a local include attack?

Re: Checking for spaces in filename when uploading a file

Posted: Thu May 01, 2008 2:46 pm
by Mordred
onion2k wrote:
Mordred wrote:Who cares how the assets are called, as long as they are accessible?
Google does. Filenames (well, urls) are a factor in your page rank when you're getting things into Google Image Search. As for a proxy script.. they work well enough, but Apache accessing the asset directly is a lot faster, especially if you're going to a database to find the asset name.
I haven't thought of google, but it's trivial to use mod_rewrite to hide the proxy script behind a pretty name.
True, the proxy script will be a bit slower, but only marginally so with a php accelerator (which any site under heavy load should use anyway, and the others will work well enough without it). Optimizing database access is another topic, but again it only applies to servers under heavy load. For assets that need pretty names (I can't imagine what user-uploaded files that would be, but let's assume there are some) you can optimize the database for string access. For other assets, just name them after the primary key.
onion2k wrote:The point here is that you're alluding to there being a security risk of some description if you use the original filename. I really don't see what risk there is so long as you remove anything that isn't alphanumeric. How is a stored image that a user has uploaded a potential vector for a local include attack?
vulnerable.php?page=../uploads/backdoor.gif

Code: Select all

include($_GET['page']);

Edit: Database-less access to secret names:

Code: Select all

//in the proxy script&#058;
$sRealName = $sSecretUploadDirBelowRoot . md5($sSecretSalt . $sPublicFileName);

Re: Checking for spaces in filename when uploading a file

Posted: Thu May 01, 2008 3:59 pm
by onion2k
Mordred wrote:vulnerable.php?page=../uploads/backdoor.gif

Code: Select all

include($_GET['page']);
Why would a developer ever try to use include() with an image? Regardless of any security issues, that would always result in a parse error. Clearly you'd be right if we were talking about general file uploads, but this thread is specifically about images. While it's sound advice, I don't think what you're talking about really pertains to the subject here.

Re: Checking for spaces in filename when uploading a file

Posted: Thu May 01, 2008 4:21 pm
by Mordred
onion2k wrote:
Mordred wrote:vulnerable.php?page=../uploads/backdoor.gif

Code: Select all

include($_GET['page']);
Why would a developer ever try to use include() with an image? Regardless of any security issues, that would always result in a parse error. Clearly you'd be right if we were talking about general file uploads, but this thread is specifically about images. While it's sound advice, I don't think what you're talking about really pertains to the subject here.
Erm, you're not following me here. Switching to verbose mode.

The only thing the developer wanted, was a lazy way to use PHP-based templates:
vulnerable.php?page=main.php (normal use)
Note that this is just a specific (and common) use case. LFIs happen in many other ways as well, and are among the top vulnerabilities in web apps.

What the attacker wants is to install a backdoor. He will be the one including the image.
He uploads a PHP file disguised as a .gif, which goes in a predictable location with a predictable name -- ../uploads/backdoor.gif
He exploits the local include vulnerability to include the malicious file. The file contains evil PHP code which does whatever the attacker wants. It will not give a parse error, it contains valid PHP code. It will also be a valid image, so it is quite relevant to the subject. Perhaps what you are missing from the picture is the idea that a file can be a valid PHP file AND a valid image. Well, yes, it can. Thus, combining two vulnerabilities, the attacker gets remote code execution on the server, well done evil guy.

With my scheme of doing file uploads it will not happen, it doesn't rely on unreliable methods like type checking, extension checking etc. I hope it's clearer now.

Re: Checking for spaces in filename when uploading a file

Posted: Thu May 01, 2008 4:44 pm
by onion2k
Mordred wrote:Perhaps what you are missing from the picture is the idea that a file can be a valid PHP file AND a valid image.
Yes, I do seem to be missing that. It sounds impossible to me. Got an example, or at least an explanation of how a file like that could work?

Re: Checking for spaces in filename when uploading a file

Posted: Thu May 01, 2008 11:49 pm
by Mordred
A file is just a series of bytes. How you interpret them depends on the interpreter. The - say- gif parser wants to see such and such header. PHP wants to see the php opening tag. You can make them both happy. You can have a file that is simultaneously a PHP script and a C source. You can have a zip that is a prime number. And so on and so on.

For a more concrete and on-topic example, here's a quote from the comments on http://ha.ckers.org/blog/20070604/passi ... imagesize/
Stefan Esser wrote: June 24th, 2007 at 2:16 am

I wonder what kind of “php” parser the original author tried…

The PHP parser will usually eat ALL characters that do not belong to the PHP script and just output them. Including 0 bytes.

You can “embedd” PHP code into any image by simply

cat my.gif my.php > evil.gif
php evil.gif
[BUMM]

And if you want to survive other thing just copy it into the palette of the gif.