How is this exploitable?

Craig321 · 28 Feb 2007 at 00:05

Hi,

First, sorry for the very messy coding, I wrote it ages ago when I didn't have much idea of neatness of coding

Anyway, can anyone see any way that this can be exploited please?:

Code:

<?PHP

error_reporting(0);

function RandString(){
$keyChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
$length = 8;


$resultKey = "";
for ($i=0;$i < $length; $i++)
  $resultKey .= substr($keyChars, rand(1, strlen($keyChars) ), 1);
  
  return $resultKey;
}

$rands = RandString();

include "db.php";

if (empty($_FILES['userfile'])) {
	echo "There was no file entered or the file you tried to upload was too big. <br><a href='index.php'>Click here</a> to return to the index.";
} else {

$validMimes = array(
    'image/png' => '.png',
    'image/x-png' => '.png',
    'image/gif' => '.gif',
    'image/jpeg' => '.jpg',
    'image/pjpeg' => '.jpg',
    'image/bmp' => '.bmp'
);

if(!array_key_exists($_FILES['userfile']['type'], $validMimes)) {
    die('Sorry, but the file type you tried to upload is invalid; only images are allowed.');
}

 // Where the file is going to be placed
$target_path = "files/";

// Add the original filename to our target path. Result is "uploads/filename.extension"
$target_path = $target_path . basename( substr($_FILES['userfile']['name'], 0, strrpos($_FILE['userfile']['name'], '.')));
$target_path .= $validMimes[$image['userfile']['type']];

$_FILES['uploadedfile']['tmp_name']; // This is how we will get the temporary file...

$target_path = "files/";

$target_path = $target_path . basename( $rands . _ . $_FILES['userfile']['name']);

if(move_uploaded_file($_FILES['userfile']['tmp_name'], $target_path)) {
     header("Location: index.php");
     //echo "There was an error uploading the file, please try again!";
}
?>
<?PHP
//-------------------- When the above is finished go onto adding it to SQL --------------------//
$filename = strip_tags(basename( $rands . _ . $_FILES['userfile']['name']));
$size = filesize($target_path);

$size = ($size > 512)?(  ($size/1024 > 512)  ?sprintf("%.02f MB",($size/1024)/1024)  :sprintf("%.02f KB",$size/1024))  :sprintf("%d B",$size);
$realfilename = basename( $_FILES['userfile']['name']);

// When submit is pressed, add it to the SQL database

	$sql = sprintf("INSERT INTO Files SET " . "File=%s, " . "IP='$REMOTE_ADDR', " . "Size=%s, " . "RealFileName=%s, " . "Date=NOW()",
	           quote_smart($filename), quote_smart($size), quote_smart($realfilename));
	
	if (mysql_query($sql)) {
		header("Location: index.php");
	} else {
		echo("<P>Error adding file: " .
			mysql_error() . "</P>");
}
	}
		
	
?>

Conrad11 · 28 Feb 2007 at 11:43

I can't see anything apparent (I am a beginner so don't take my word).

Seems ok to me.

Craig321 · 28 Feb 2007 at 14:08

Yea, looks fine to me as well... it's too messy for my liking, but it works and I can't really be bothered to re-write it at the moment.

Someone managed to upload a PHP file through it

Conrad11 · 28 Feb 2007 at 16:23

Surely if you rename with an extension based on a image mime type, then the php script would not get executed.

I thought you were doing that as i scanned through it but it appears you are not.

Craig321 · 1 Mar 2007 at 01:53

Conrad11 said:
Surely if you rename with an extension based on a image mime type, then the php script would not get executed.

I thought you were doing that as i scanned through it but it appears you are not.

I could do that, but the script I currently have should only allow those types which I have allowed, so I shouldn't really need to.

Is there any other possible way anyone could upload a PHP file through that?

EDIT

Ok, here's another possibility.

My news page:

Code:

		    <?PHP
include "db_1.php";

$id = strip_tags($_GET[id]);

if(!intval($id))
{
  echo 'ID cannot be letters';
}
else
{

			$query = sprintf("SELECT * FROM News WHERE ID = %s", quote_smart($id));  //2nd query
			$newslist = @mysql_query($query); //running it
			$news = mysql_fetch_array($newslist); //displaying it 

if(mysql_num_rows($newslist)<1)
{
  echo 'Invalid ID';
}
else
{
		
		if ($newslist) { //if it ran ok
			$id = strip_tags($news["ID"]);
			$title = strip_tags($news["Title"]);
			$content = strip_tags($news["Content"], '<br>');
			$date = strip_tags($news["Date"]);

		if(($content =="")){
			echo "<br><strong><center>Incorrect news ID specified</center></strong>";
		} else {
			echo "<table width='550' border='1' cellspacing='0' cellpadding='5'>
              <tr>
                <td bgcolor='#CCCCCC'><b>News item: $id &nbsp; &nbsp; Title: $title &nbsp; &nbsp; Posted: $date</b></td>
              </tr>
              <tr>
                <td>$content</td>
              </tr>
            </table><br><br><br><br><br><br>";
		}
		
		} else { //didnt run
			echo("<P>Error loading news item: " .
			mysql_error() . "</P>");
		}

}
}
?>

Again, sorry, it's very messy

Is there any way that could have been exploited to e.g. let someone use their own upload.php file on the uploader?

Thanks,
Craig.