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
Javrixx
Forum Commoner
Posts: 32 Joined: Thu Aug 24, 2006 2:05 pm
Post
by Javrixx » Thu Aug 24, 2006 2:09 pm
Everah | Please use 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]
I've been searching for 3 days and can't find the answer. Either the code I put in doesn't work or I'm a moron. I am pretty new to PHP, so that could be a major issue. What I have setup is a page where people can upload images. This works just fine, the script as of right now works perfect, no problems.
But after reading around, I found people can just rename a .php or some other file to just a .jpg or .gif, upload it, and then execute it... I do not want this to happen!
So the solution everyone is talking about is using one of two codes to verify the file is actually an image that is being uploaded.
getimagesize() and exif_imagetype() are the two codes. I can't get either to work. I'm just learning PHP for the first time, so I'm very unfamiliar with it. What I need to know is... what code do I use and where do I put it in my script so it will verify the file is really an image. I don't care which one is used. The exif is supposed to be faster, but I don't really care.
Below is my script. Thanks!Code: Select all
<?
$num_of_uploads=1;
$file_types_array=array("jpg","gif");
$max_file_size=110000;
$upload_dir="images_public/";
function uploaderFILES($num_of_uploads=1, $file_types_array=array("jpg","gif"), $max_file_size=110000, $upload_dir="images_public/"){
if(!is_numeric($max_file_size)){
$max_file_size = 110000;
}
foreach($_FILES["file"]["error"] as $key => $value)
{
if($_FILES["file"]["name"][$key]!="")
{
if($value==UPLOAD_ERR_OK)
{
$origfilename = $_FILES["file"]["name"][$key];
$filename = explode(".", $_FILES["file"]["name"][$key]);
$filenameext = $filename[count($filename)-1];
unset($filename[count($filename)-1]);
$filename = implode(".", $filename);
$filename = substr($filename, 0, 15).".".$filenameext;
$file_ext_allow = FALSE;
if (file_exists('images_public/' . $filename)) {
$tmpVar = 1;
while(file_exists('images_public/' . $tmpVar . '-' . $filename)) {
$tmpVar++;
}
$filename= $tmpVar . '-' . $filename;
}
for($x=0;$x<count($file_types_array);$x++){
if($filenameext==$file_types_array[$x])
{
$file_ext_allow = TRUE;
}
}
if($file_ext_allow){
if($_FILES["file"]["size"][$key]<$max_file_size){
if(move_uploaded_file($_FILES["file"]["tmp_name"][$key], $upload_dir.$filename)){
echo("<center>File uploaded successfully. Your image can be found at <a href='http://www.averageguysteve.com/".$upload_dir.$filename."' target='_blank'>http://www.averageguysteve.com/".$upload_dir.$filename."</a><br /><br /><br /><img src='".$upload_dir.$filename."' border='0' alt=''></center>");
}
else { echo('<center><font color="#FF0000">'.$origfilename."</font> was not successfully uploaded.<br /></center>");}
}
else { echo('<center><font color="#FF0000">'.$origfilename."</font> was too big and was not uploaded. Max file size is 100k!<br /></center>"); }
}
else{ echo('<center><font color="#FF0000">'.$origfilename." </font>had an invalid file extension and was not uploaded. Valid file types are .jpg or .gif.<br /></center>"); }
}
else{ echo('<center><font color="#FF0000">'.$origfilename." </font>was not successfully uploaded.<br /></center>"); } // else
}
}
}
?>
<HTML>
<BODY>
<FORM action='<?=$PHP_SELF;?>' method='post' enctype='multipart/form-data'>Upload file:<BR /><INPUT type='hidden' name='submitted' value='TRUE' id='<?=time();?>' >
<INPUT type='hidden' name='MAX_FILE_SIZE' value='<?=$max_file_size;?>' >
<? for($x=0;$x<$num_of_uploads;$x++){
$form .= "<input type='file' name='file[]'><br />";
}
$form .= "<input type='submit' value='Upload'><br /><br />
<font color='red'>*</font>Max file size is 100k. Valid file types are .";
for($x=0;$x<count($file_types_array);$x++){
if($x<count($file_types_array)-1){
$form .= $file_types_array[$x]." or .";
}else{
$form .= $file_types_array[$x].".";
}
}
echo($form);
?>
</FORM>
</BODY>
</HTML>
<?
if(isset($_POST["submitted"])){
uploaderFILES($num_of_uploads, $file_types_array, $max_file_size, $upload_dir);
}
?>
Everah | Please use 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]
volka
DevNet Evangelist
Posts: 8391 Joined: Tue May 07, 2002 9:48 am
Location: Berlin, ger
Post
by volka » Fri Aug 25, 2006 12:46 pm
pickle
Briney Mod
Posts: 6445 Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:
Post
by pickle » Fri Aug 25, 2006 2:12 pm
PHP Manual wrote: If accessing the filename image is impossible, or if it isn't a valid picture, getimagesize() will return FALSE and generate an error of level E_WARNING.
I'd guess you'd put it in your code here:
Code: Select all
...
if($filenameext==$file_types_array[$x] || !@getimagesize($filename))
{
$file_ext_allow = TRUE;
}
...
FYI: The '@' will suppress the possible warning getimagesize() may generate.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
feyd
Neighborhood Spidermoddy
Posts: 31559 Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA
Post
by feyd » Fri Aug 25, 2006 5:11 pm
Avoid the "@" operator.
Code: Select all
$err = error_reporting(0);
$info = getimagesize($filename);
error_reporting($err);
William
Forum Contributor
Posts: 332 Joined: Sat Oct 25, 2003 4:03 am
Location: New York City
Post
by William » Sat Aug 26, 2006 12:49 am
feyd wrote: Avoid the "@" operator.
Code: Select all
$err = error_reporting(0);
$info = getimagesize($filename);
error_reporting($err);
[at] feyd
I was just wondering what the reasoning for avoiding the "@" operator is?
feyd
Neighborhood Spidermoddy
Posts: 31559 Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA
Post
by feyd » Sat Aug 26, 2006 12:51 am
It's slower than the posted snippet.
RobertGonzalez
Site Administrator
Posts: 14293 Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA
Post
by RobertGonzalez » Sat Aug 26, 2006 10:13 am
Not to mention that if there is a potential for the error, you can code error trapping/checking mechanisms into the code to handle this, instead of just quieting them. This makes for more extensible, potable, efficient code.
Javrixx
Forum Commoner
Posts: 32 Joined: Thu Aug 24, 2006 2:05 pm
Post
by Javrixx » Wed Sep 06, 2006 4:51 pm
Sorry guys, I don't know if I'm just retarded or what, but both of what you posted below doesn't seem to work. I can still upload non-images that just have a .jpg extension... Again, I'm really new at this so please be a little more basic with me as far as the code goes.
feyd
Neighborhood Spidermoddy
Posts: 31559 Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA
Post
by feyd » Wed Sep 06, 2006 4:53 pm
What's the new code?
Javrixx
Forum Commoner
Posts: 32 Joined: Thu Aug 24, 2006 2:05 pm
Post
by Javrixx » Thu Sep 07, 2006 12:45 pm
Actually I got it fixed. I guess my host doesn't support the exif command because I just get a fatal error when using it, but the imagesize code works fine. Here is the new code, only one line is altered really and just added the extra closing bracket. But now my error messages aren't showing up correctly, I'm going to stare at the code and try to soak it up now, see why it doesn't display propery.
The actually page is
http://www.averageguysteve.com/upload.php
Code: Select all
<?
$num_of_uploads=1;
$file_types_array=array("jpg","gif");
$max_file_size=110000;
$upload_dir="images_public/";
function uploaderFILES($num_of_uploads=1, $file_types_array=array("jpg","gif"), $max_file_size=110000, $upload_dir="images_public/"){
if(!is_numeric($max_file_size)){
$max_file_size = 110000;
}
foreach($_FILES["file"]["error"] as $key => $value)
{
if($_FILES["file"]["name"][$key]!="")
{
if($value==UPLOAD_ERR_OK)
{
$origfilename = $_FILES["file"]["name"][$key];
$filename = explode(".", $_FILES["file"]["name"][$key]);
$filenameext = $filename[count($filename)-1];
unset($filename[count($filename)-1]);
$filename = implode(".", $filename);
$filename = substr($filename, 0, 15).".".$filenameext;
$file_ext_allow = FALSE;
if (file_exists('images_public/' . $filename)) {
$tmpVar = 1;
while(file_exists('images_public/' . $tmpVar . '-' . $filename)) {
$tmpVar++;
}
$filename= $tmpVar . '-' . $filename;
}
for($x=0;$x<count($file_types_array);$x++){
if($filenameext==$file_types_array[$x])
{
$file_ext_allow = TRUE;
}
}
if($file_ext_allow){
if($_FILES["file"]["size"][$key]<$max_file_size){
if(getimagesize($_FILES["file"]["tmp_name"][$key])){
if(move_uploaded_file($_FILES["file"]["tmp_name"][$key], $upload_dir.$filename)){
echo("<center>File uploaded successfully. Your image can be found at <a href='http://www.averageguysteve.com/".$upload_dir.$filename."' target='_blank'>http://www.averageguysteve.com/".$upload_dir.$filename."</a><br /><br /><br /><img src='".$upload_dir.$filename."' border='0' alt=''></center>");
}
else { echo('<center><font color="#FF0000">'.$origfilename."</font> was not successfully uploaded.<br /></center>");}
}
else { echo('<center><font color="#FF0000">'.$origfilename."</font> was too big and was not uploaded. Max file size is 100k!<br /><br /><b>OR</b><br /><br />Your file was not a real image file. Only images are allowed!<br /></center>"); }
}
else{ echo('<center><font color="#FF0000">'.$origfilename." </font>had an invalid file extension and was not uploaded. Valid file types are .jpg or .gif.<br /></center>"); }
}
else{ echo('<center><font color="#FF0000">'.$origfilename." </font>was not successfully uploaded.<br /></center>"); } // else
}
}
}
}
?>
<HTML>
<TITLE>AverageGuySteve.com - Upload an Image</TITLE>
<BODY>
<FORM action='<?=$PHP_SELF;?>' method='post' enctype='multipart/form-data'>Upload file:<BR /><INPUT type='hidden' name='submitted' value='TRUE' id='<?=time();?>' >
<INPUT type='hidden' name='MAX_FILE_SIZE' value='<?=$max_file_size;?>' >
<? for($x=0;$x<$num_of_uploads;$x++){
$form .= "<input type='file' name='file[]'><br />";
}
$form .= "<input type='submit' value='Upload'><br /><br />
<font color='red'>*</font>Max file size is 100k. Valid file types are .";
for($x=0;$x<count($file_types_array);$x++){
if($x<count($file_types_array)-1){
$form .= $file_types_array[$x]." or .";
}else{
$form .= $file_types_array[$x].".";
}
}
echo($form);
?>
</FORM>
</BODY>
</HTML>
<?
if(isset($_POST["submitted"])){
uploaderFILES($num_of_uploads, $file_types_array, $max_file_size, $upload_dir);
}
?>
Thanks again for your help guys. Any other suggestions or comments is greatly appreciated as I'm so new to this php thing.