Skip to content

File upload validation constraints (maxFiles, maxSize, fileTypes) are silently ignored #201

Description

@rvanbaalen

Summary

FieldValidator declares maxFiles, maxSize, and fileTypes as validation rules for VFORM_FILE fields, including dedicated error-message strings (__maxfileserror, __maxsizeerror, __filetypeerror). Developers are expected — and led — to use these to enforce server-side upload constraints:

$form->addField(
    'document',
    'Upload document',
    ValidForm::VFORM_FILE,
    [
        'required'  => true,
        'maxFiles'  => 1,
        'maxSize'   => 100,                       // KB
        'fileTypes' => ['application/pdf'],       // PDF only
    ]
);

Nothing in the codebase ever reads those properties. The rules are mass-assigned into the validator via the constructor's property_exists loop, and then never consulted again. FieldValidator::validate() doesn't look at $_FILES, doesn't call is_uploaded_file(), doesn't check file counts, sizes, MIME types, or extensions, and doesn't emit any of the three error messages listed above. The File class itself doesn't check them either — it only renders HTML and JS.

The practical result is 100% bypass of every server-side file upload constraint the library offers. Developers get a false sense of security from a declared API that the library silently refuses to enforce.

Reachability

$ grep -rn 'getMaxFiles\|getMaxSize\|getFileTypes\|__maxfiles\|__maxsize\|__filetypes' classes/
classes/ValidFormBuilder/FieldValidator.php:53: * @method integer getMaxFiles() …
classes/ValidFormBuilder/FieldValidator.php:54: * @method void setMaxFiles() …
classes/ValidFormBuilder/FieldValidator.php:55: * @method integer getMaxSize() …
classes/ValidFormBuilder/FieldValidator.php:56: * @method void setMaxSize() …
classes/ValidFormBuilder/FieldValidator.php:57: * @method array getFileTypes() …
classes/ValidFormBuilder/FieldValidator.php:58: * @method void setFileTypes() …
classes/ValidFormBuilder/FieldValidator.php:166: protected $__maxfiles = 1;
classes/ValidFormBuilder/FieldValidator.php:172: protected $__maxsize = 3000;
classes/ValidFormBuilder/FieldValidator.php:178: protected $__filetypes;
classes/ValidFormBuilder/FieldValidator.php:241: protected $__maxfileserror = …;
classes/ValidFormBuilder/FieldValidator.php:246: protected $__maxsizeerror = …;

Every match is either the property declaration or the @method docblock. Zero matches for actual read/enforcement sites. Likewise:

$ grep -rn '\$_FILES\|UPLOAD_ERR\|is_uploaded_file\|move_uploaded_file' classes/
classes/ValidFormBuilder/FieldValidator.php:345: * … the return value is the $_FILES[fieldname] array.

The only reference to $_FILES is in a docblock comment, not in executable code.

Proof of concept

<?php
require 'vendor/autoload.php';
use ValidFormBuilder\ValidForm;

// Developer declares strict upload constraints.
$form = new ValidForm('upload-form');
$field = $form->addField(
    'document',
    'Upload document',
    ValidForm::VFORM_FILE,
    [
        'required'  => true,
        'maxFiles'  => 1,                     // one file only
        'maxSize'   => 100,                   // 100 KB maximum
        'fileTypes' => ['application/pdf'],   // PDF only
    ]
);
$validator = $field->getValidator();

// Simulate an attacker upload: 50 files, 1 GB each, all .exe.
$_FILES['document'] = [
    'name'     => array_fill(0, 50, 'evil.exe'),
    'type'     => array_fill(0, 50, 'application/x-msdownload'),
    'tmp_name' => array_fill(0, 50, '/tmp/fake'),
    'error'    => array_fill(0, 50, UPLOAD_ERR_OK),
    'size'     => array_fill(0, 50, 1_073_741_824),
];
$_REQUEST['document'] = array_fill(0, 50, 'evil.exe');

var_dump($validator->validate());   // bool(true)  — constraint bypass
var_dump($validator->getError());   // string(0) "" — no error

Output:

bool(true)
string(0) ""

The validator reports the submission as valid even though all three declared constraints (count, size, MIME type) are violated.

Impact

  • Any developer using VFORM_FILE with maxFiles, maxSize, or fileTypes is affected. These are the three named constraints the library documents for file fields.
  • Arbitrary file upload: attackers can upload files of any MIME type, bypassing whitelists. Combined with a downstream MIME-sniffing code path or a web server that serves uploads directly, this enables arbitrary file execution.
  • Disk exhaustion DoS: attackers can upload arbitrarily large files and arbitrarily many of them, limited only by PHP's upload_max_filesize / post_max_size / max_file_uploads ini settings. If those limits are generous or unset, a single request can fill the server disk.
  • Silent failure: the library appears to work. Developers who read the documented validation rules and trust them get no warning, no deprecation notice, no runtime error. They will only discover the gap during a security audit or after a breach.
  • Regression against the documented API: The docblock explicitly says @method integer getMaxSize() … getMaxSize() Returns the value of $__maxsize, and the error string __maxsizeerror is "The filesize is too big. The maximum is %s KB." — the library promises a feature it doesn't ship.

Expected fix

FieldValidator::validate() needs a file-upload branch (or a new FileValidator class) that runs for VFORM_FILE fields and:

  1. Reads $_FILES[$fieldname] instead of $_REQUEST[$fieldname].
  2. Verifies each entry via is_uploaded_file($tmp_name) to prevent path traversal via the tmp_name key.
  3. Rejects any entry whose error is not UPLOAD_ERR_OK (mapping the ini-based errors to the declared maxSize / maxFiles errors where appropriate).
  4. Counts uploaded files and compares against $__maxfiles; error message is __maxfileserror.
  5. Compares each file's size (in bytes) against $__maxsize (documented as KB — multiply by 1024); error message is __maxsizeerror.
  6. Verifies each file's MIME type by calling finfo_file() on tmp_name (never trust the client-sent type field — that's attacker-controlled) and matches against $__filetypes; error message is __filetypeerror.
  7. On the successful path, exposes the uploaded files via getValidValue() so downstream code can move them to permanent storage.

A minimal implementation might look like (pseudocode):

if ($this->__type === ValidForm::VFORM_FILE) {
    return $this->validateFileUpload($intDynamicPosition);
}

…where validateFileUpload runs the pipeline above. This is a new feature-sized change, not a one-line patch — but the absence of it is a security regression against the documented API surface.

Mitigation for application developers until the fix lands

Do not rely on maxFiles, maxSize, or fileTypes from ValidFormBuilder for server-side enforcement. Write your own post-form check:

if ($form->isValid()) {
    $files = $_FILES['document'];
    if (count((array) $files['name']) > 1) { /* reject */ }
    foreach ((array) $files['size'] as $size) {
        if ($size > 100 * 1024) { /* reject */ }
    }
    foreach ((array) $files['tmp_name'] as $i => $tmp) {
        if (!is_uploaded_file($tmp)) { /* reject */ }
        $mime = (new finfo(FILEINFO_MIME_TYPE))->file($tmp);
        if ($mime !== 'application/pdf') { /* reject */ }
    }
    // only now, trust the upload
}

Discovered while writing comprehensive unit tests for File in #154.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions