patternphpMinor
Image, file, text uploader and URL shortener
Viewed 0 times
fileimageshortenertextuploaderandurl
Problem
I've made a tiny PHP script that sits on my server and receives images/files and text/urls I send to it from a desktop application. I'm a bit worried about how secure the whole thing is because the upload endpoint is the same URL as users who view the content I've uploaded go to.
This script takes uploaded things and stores them in the database under a unique, random, 5-character alphanumeric string. Text and URLs are stored verbatim (albeit
The database credentials (
index.php:
And should it matter, .htaccess:
This script takes uploaded things and stores them in the database under a unique, random, 5-character alphanumeric string. Text and URLs are stored verbatim (albeit
mysqli_escape_string'd) in the database while files (this includes images) are stored in a subfolder named "upload" that the webserver user has 0755 access to. URLs are location redirected, text is displayed raw, and files are fpassthrough()ed when a user requests an existing 5-character string.The database credentials (
$db_) and upload control variables ($upload_) are in uploader_settings.inc.php (which is not included here).index.php:
0) {
$array = mysqli_fetch_array($query);
$value = $array['value'];
switch ($array['type']) {
case "url":
header("Location: ".$value);
die();
case "text":
echo $value;
die();
case "file":
header("Content-Type: ".mime_content_type($value));
fpassthru(fopen($value,"rb"));
die();
}
} else {
header("Location: ".$upload_failRedirect);
die();
}
}
function random_str($length, $keyspace = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ')
{
$str = '';
$max = mb_strlen($keyspace, '8bit') - 1;
for ($i = 0; $i < $length; ++$i) {
$str .= $keyspace[random_int(0, $max)];
}
return $str;
}And should it matter, .htaccess:
RewriteEngine on
#all files and directories
RewriteCond %{REQUEST_FILENAME} !-f [OR]
RewriteCond %{REQUEST_FILENAME} !-d
#pass to index as perameter
RewriteRule ^(.*)$ index.php?selector=$1 [NC,L,QSA]Solution
Use prepared statements
This, and other SQL statements in your code, is an SQL injection waiting to happen:
Don't construct SQL by embedding user input into a string.
Use prepared statements.
Don't store clear passwords
This suggests that you have somewhere a password stored in clear text:
It's better to store passwords in a hashed form,
so that they cannot be stolen too easily.
This, and other SQL statements in your code, is an SQL injection waiting to happen:
mysqli_query($conn,"INSERT INTO ".$db_table." (shortened,type,value) VALUES ('".$random."','".$_POST['type']."','".mysqli_real_escape_string($conn,$value)."');");Don't construct SQL by embedding user input into a string.
Use prepared statements.
Don't store clear passwords
This suggests that you have somewhere a password stored in clear text:
if ($_POST['pass'] != $upload_pass) {It's better to store passwords in a hashed form,
so that they cannot be stolen too easily.
Code Snippets
mysqli_query($conn,"INSERT INTO ".$db_table." (shortened,type,value) VALUES ('".$random."','".$_POST['type']."','".mysqli_real_escape_string($conn,$value)."');");if ($_POST['pass'] != $upload_pass) {Context
StackExchange Code Review Q#157487, answer score: 4
Revisions (0)
No revisions yet.