Several SQL injection and file upload vulnerabilities

[LioTree]

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

SQL Injection

HistoryController::getList

https://github.com/xiebruce/PicUploader/blob/2ef5b21cb2ae831ff5a56473b0f1315a6f81ff65/settings/HistoryController.php#L107

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

HistoryController::getByConditions

https://github.com/xiebruce/PicUploader/blob/2ef5b21cb2ae831ff5a56473b0f1315a6f81ff65/settings/HistoryController.php#L189

The variables $key and $val come from the parameter $conditionArr, and users can control the values of this parameter through https://github.com/xiebruce/PicUploader/blob/2ef5b21cb2ae831ff5a56473b0f1315a6f81ff65/settings/dispatch.php#L30

HistoryModel.php::createOne

https://github.com/xiebruce/PicUploader/blob/2ef5b21cb2ae831ff5a56473b0f1315a6f81ff65/settings/HistoryModel.php#L31

Similar to the previous one, Users can control $data through https://github.com/xiebruce/PicUploader/blob/2ef5b21cb2ae831ff5a56473b0f1315a6f81ff65/settings/dispatch.php#L30

File Upload

SettingController::uploadFile

https://github.com/xiebruce/PicUploader/blob/2ef5b21cb2ae831ff5a56473b0f1315a6f81ff65/settings/SettingController.php#L456

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

https://github.com/xiebruce/PicUploader/blob/2ef5b21cb2ae831ff5a56473b0f1315a6f81ff65/index.php#L97

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.