Several SQL injection and file upload vulnerabilities

[LioTree]

Hi, I would like to report some serious security vulnerabilities.

SQL Injection

HistoryController::getList

PicUploader/settings/HistoryController.php

Line 107 in 2ef5b21

$where = '((`filename` LIKE "%'.$keyword.'%") OR (`url` LIKE "%'.$keyword.'%") OR (`created_at` LIKE "%'.$keyword.'%"))';

The variable $keyword is directly controlled by $_GET['keyword'], which allows an attacker to inject SQL statements.

HistoryController::getByConditions

PicUploader/settings/HistoryController.php

Line 189 in 2ef5b21

$where .= "`".$key."`='".$val."' AND ";

The variables $key and $val come from the parameter $conditionArr, and users can control the values of this parameter through

PicUploader/settings/dispatch.php

Line 30 in 2ef5b21

$json = call_user_func_array(array((new $className()), $func), [$_REQUEST]);

HistoryModel.php::createOne

PicUploader/settings/HistoryModel.php

Line 31 in 2ef5b21

$sql = 'INSERT INTO `'.self::$tableName.'`(`filename`, `url`, `size`, `created_at`, `mime`, `md5`, `upload_server`) VALUES("'.$data['filename'].'", "'.$data['url'].'", '.$data['size'].', "'.$uploadTime.'", "'.$data['mime'].'", "'.$data['md5'].'", "'.$data['uploadServer'].'")';

Similar to the previous one, Users can control $data through

PicUploader/settings/dispatch.php

Line 30 in 2ef5b21

$json = call_user_func_array(array((new $className()), $func), [$_REQUEST]);

File Upload

SettingController::uploadFile

PicUploader/settings/SettingController.php

Line 456 in 2ef5b21

$newName = md5(uniqid(microtime(true))).$ext;

The type of file extension for uploads is not restricted, and the path after uploading is directly returned to the user. Attackers can upload malicious PHP files.

index.php

PicUploader/index.php

Line 97 in 2ef5b21

if(move_uploaded_file($tmp_name, $dest)){

There is also no restriction on the file extension of the uploaded files. Although the temporary file will be deleted later, attackers can access the uploaded malicious PHP file through race conditions.

[xiebruce]

Thank you for you report.

about keyword in "HistoryController::getList", I add this line

$keyword = addslashes($keyword);

about HistoryController::getByConditions, I add this

$where .= "`".addslashes($key)."`='".addslashes($val)."' AND ";

about PicUploader/settings/dispatch.php, I don't know how to modify it.

about HistoryModel.php::createOne, I add this

foreach($data as &$val){
	$val = addslashes($val);
}

about SettingController::uploadFile, I add this

if ($ext == ".php"){
	return false;
}

about index.php I add this

$ext = mb_substr($files['name'][$key], strrpos($files['name'][$key], '.'));
if (strtolower($ext) == '.php'){
	continue;
}

and this

	$ext = mb_substr($files['name'], strrpos($files['name'], '.'));
	if (strtolower($ext) != '.php'){
		if(move_uploaded_file($tmp_name, $dest)){
			$argv[] = $dest;
		}
	}

add also add this in dashboard.js

  if (file.name.lastIndexOf(".php") > -1) {
    setTimeout(function () {
      // means not allow uploading php file
      alert("不允许上传php文件")
    }, 500)
    setTimeout(function () {
      $(".un-uploaded").remove()
    }, 1000)
    return false
  }

[LioTree]

Hi, simply prohibiting .php cannot fully solve the file upload vulnerability, as some servers also support parsing extensions like .phtml, .php5, .pht, etc. If possible, it's best to set a whitelist for file extensions.

[xiebruce]

Actually this tool is for personal use, if you use on local machine, you don't need to worry about what file you're uploading.

---

If you deploy it on remote server, you need to set a HTTP Authentication in nginx config

auth_basic 'Restricted'; 
auth_basic_user_file /usr/local/nginx/htpasswd; 

use this command to generate htpasswd

htpasswd -bc /usr/local/nginx/htpasswd foobar 123456

we visit our Picuploader through domain, and Picuploader is behind nginx, user cannot visit Picuploader directly, and actually the user is yourself too(only yourself), because this tool is designed for personal use, so you don't need to worry about what type of file you're uploading, attackers can not visit your Picuploader, so even if you upload a php file, attackers still can't visit it.

[LioTree]

Hi, thanks for your reply. I understand your point, but I think these vulnerabilities are worth addressing. Even if PicUploader is deployed on a local machine, attackers could potentially exploit these vulnerabilities to gain access to the user's computer when they are on the same internal network. Of course, this depends on how users configure their web servers on their local machines, but there is indeed a certain level of risk. Even if users enable authentication on nginx, they may set weak passwords, leading to similar risks.