# Upload image with GD library

First of all I just started learning OOP.

My priority is to make safe code. I think my code is safe, because I use GD to make a copy of the uploaded image and I use an image proxy to make sure nobody can access the uploaded file directly and it's uploaded outside of the webroot. But if I was 100% sure that it's safe I was not posting it here haha. I'm not sure about the way I check the MIME type, and about the GD.

I made some tests and it works fine, but what do you guys think? Any suggestions are welcome. Also if you have any questions about the code just ask me in the comments.

Class Img:

class Img
{
private static $folder,$file, $ext,$w, $h; public static function upload($file, $new_size){ self::$ext = pathinfo($file["name"], PATHINFO_EXTENSION); list(self::$w, self::$h) = ($size = getimagesize($file["tmp_name"])); if(!$size || !in_array(self::$ext, ['png', 'jpeg', 'jpg']))die(json_encode(['error' => 'Invalid file'])); if(filesize($file["tmp_name"]) > 2700000)die(json_encode(['error' => 'File is too big']));

self::$folder = dirname(__DIR__).'/../../uploads/profile/'.$_SESSION['user_folder'];
self::$file =$file["tmp_name"];

$_SESSION['avatar'] = ($name = mt_rand(100, 100000).'-'.time().'.'.self::$ext); self::resize_image($name, $new_size,$new_size);

$user_id =$_SESSION['user_id'];
$conn = \lib\Db::instance();$query = $conn->query("UPDATE users SET avatar = '{$name}' WHERE user_id = '{$user_id}'");$conn = NULL;

die(json_encode(['avatar' => $_SESSION['avatar']])); } private static function resize_image($name, $width,$height){
if(self::$ext === 'jpeg')self::$ext = 'jpg';

if(self::$ext === 'jpg')$img = imagecreatefromjpeg(self::$file); elseif(self::$ext === 'png')$img = imagecreatefrompng(self::$file);

$ratio = max($width/self::$w,$height/self::$h);$x = (self::$w -$width/$ratio)/2; self::$h = $height/$ratio;
self::$w =$width/$ratio;$new = imagecreatetruecolor($width,$height);

// preserve transparency
if(self::$ext === "png"): imagecolortransparent($new, imagecolorallocatealpha($new, 0, 0, 0, 127)); imagealphablending($new, false);
imagesavealpha($new, true); endif; imagecopyresampled($new, $img, 0, 0,$x, 0, $width,$height, self::$w, self::$h);

if(self::$ext === 'jpg')imagejpeg($new, self::$folder.$name);
elseif(self::$ext === 'png')imagepng($new, self::$folder.$name);
imagedestroy($new); } }  avatar_upload.php: require_once dirname(__DIR__).'/../../vendor/autoload.php'; if(!isset($_SESSION))session_start();
if($_SERVER['REQUEST_METHOD'] != 'POST' || empty($_SESSION['user_id']))die(header('HTTP/1.1 404 Not Found'));
if(!empty($_FILES['avatar']['size']))\lib\Img::upload($_FILES["avatar"], 150);


• Instead of murdering your script with die() calls, return a consistent data type no matter the outcome. In the future, you may change how you want to display/deliver the results. Finally, convert your returned array to json and echo.
• Avoid single-use and unnecessary variable declarations. If you have $_SESSION['avatar'] then you don't need to declare $name.
• You said that i should use prepared statements but is it because of the variable $ext? I mean, the variable $user_id is from a column with AUTO_INCREMENT, and the $name is made with the function mt_rand(), so i don't see other reason to use prepared statements. I only use it when there's user input. – mario Jul 15 '19 at 21:53 • I can't see the code where $_SESSION['user_id'] is generated. If it is at all insecure, then it becomes a potential attack vector for this script. If it may possibly contain characters that will monkeywrench your query, then it becomes a potential point of breakage. Regardless of how stable/secure you believe the SESSION data is, I recommend consistently using prepared statements to write variables into your queries. If the $_SESSION['user_id'] is an integer (and you are not going to use a prepared statement) you could use explicit casting ((int)$_SESSION['user_id']). – mickmackusa Jul 15 '19 at 22:48
• in the php.ini i had set sessions to httponly and secure. The user id is generated in the database with AUTO_INCREMENT, so i think it's safe. But i have a question, do you trust the value from the variable \$ext? I meant, not to check the MIME type, but to use it in the name of the file and in the sql (which i did), I made a check with the function in_array, but i don't know if it's enough to trust. – mario Jul 16 '19 at 0:43