CVE-2019-16790 (PHP, published 2019-12-30T20:15Z)
Description: value: In Tiny File Manager before 2.3.9, there is a remote code execution via Upload from URL and Edit/Rename files. Only authenticated users are impacted.
Severity: MEDIUM (CVSS v3 base score: 8.8)
Weakness: CWE-434 \u2014 Unrestricted Upload of File with Dangerous Type
The product allows the upload or transfer of dangerous file types that are automatically processed within its environment.
Repository: https://github.com/prasathmani/tinyfilemanager
Files changed:
tinyfilemanager.php
Commit message: Merge pull request from GHSA-w72h-v37j-rrwr

* Fix the RCE vuln via Upload from URL

This commit attemps to fix the Remote Code Execution
(authenticated) via Upload from URL. Some notes about
the proposed solution:

* A new function (fm_is_file_allowed) has been created to
validate if the filename is allowed. This function gets the
the filename as parameter and returns true if it validates
as allowed. Otherwise returns false (the default).

* It's better to have such validatation(s) in one place
instead of spread all over the code. There are other places in
the application where the filename is validated and they should
all be refactored to call this function. Then we can focus
all needed validations in one place only!

NOTE: This refactoring was not done - the only goal was to fix
this security vulnerability only.

* The fm_is_file_allowed() function validates the filename
based on its extension only. No other validatation(s) have been
implemented in this commit.

* File extensions are assumed to be case-insensitive.
For example, php == PHP == Php == PhP, etc. This is consitent
with some web servers. Without this, the user will have to populate
the $allowed_extensions with all possible allowed combinations.

* Although, there is one drawback to the current solution, which
is that all files must have an extension to be uploaded. This is not
consitent with modern filesystems. Maybe a better solution would be
to automatically append an extension to the filename if no
extension has been found (e.g., .html or .txt which are generally
considered to be harmless). This must be decided by the
application's maintainers.

* Fix the RCE vulns via new/rename file

Sanitize the arguments to stat using escapeshellarg()

Co-authored-by: Jorge Morgado <jorge@morgado.ch>


Vulnerable code:
define('VERSION', '2.3.8');
        $cmd = ($iswin) ? "for %F in (\"$file\") do @echo %~zF" : ($isdarwin ? "stat -f%z \"$file\"" : "stat -c%s \"$file\"");

Fixed code:
define('VERSION', '2.3.9');
        if(!fm_is_file_allowed($fileinfo->name)) {
            $err = array("message" => "File extension is not allowed");
            event_callback(array("fail" => $err));
            exit();
        }

/**
 * Check if the filename is allowed.
 * @param string $filename
 * @return bool
 */
function fm_is_file_allowed($filename)
{
    // By default, no file is allowed
    $allowed = false;

    if (FM_EXTENSION) {
        $ext = strtolower(pathinfo($filename, PATHINFO_EXTENSION));

        if (in_array($ext, explode(',', strtolower(FM_EXTENSION)))) {
            $allowed = true;
        }
    }

    return $allowed;
}

        $arg = escapeshellarg($file);
        $cmd = ($iswin) ? "for %F in (\"$file\") do @echo %~zF" : ($isdarwin ? "stat -f%z $arg" : "stat -c%s $arg");