From f0f471bc1e0aa94cc32539520038f6411a90f846 Mon Sep 17 00:00:00 2001 From: Jake Jackson Date: Wed, 19 Aug 2026 16:14:02 +1000 Subject: [PATCH 1/2] Security hardening for 4.0.0 Storage writes each upload to an unguessable staged name inside the destination directory and renames it into place, rather than writing to the destination directly. move_uploaded_file() falls back to a stream copy across file systems and that copy follows a symlink at the destination; rename() replaces the directory entry instead of following it, so the bytes never travel through a link someone planted and no partial content is readable under the final name. An extension deny-list is on by default -- 59 extensions a server executes plus 15 a browser renders -- checked against every dot-separated component, because Apache's AddHandler matches any component and serves evil.php.jpg as PHP. Storage also refuses traversal, dotfiles, C0/C1 controls, DEL, the bidi and zero-width characters that make a stored name render as something it is not, Windows device names, and a symlinked destination. Stored files get mode 0640. upload() now requires something to validate against. Validation\FileType pairs an extension with the media type sniffed from the contents and requires the two to agree. Validation\Extension and Validation\Mimetype check independent allow-lists, so a GIF named avatar.png satisfies both; they are deprecated but still work. Filenames and extensions are sanitized, including in the strings getErrors() returns. setExtension() discards an invalid extension whole rather than stripping characters from it, because stripping turned avatar.p-h-p into a stored, executable avatar.php. GravityPdf\Upload\Filename holds the rules for what counts as a usable filename. Two layers apply them to different ends -- FileInfo rewrites a client-supplied name, storage refuses one that still breaks a rule, because FileInfoInterface is a public extension point -- and that split is deliberate. The two disagreeing about what the rules are was not: both control-character filters covered C0 alone and each had to be fixed separately. The constants and the splitters are declared once now, and neither layer splits a filename for itself. Breaking changes callers must act on, in full in UPGRADE.md: '5MB' parsed as 5 bytes and now parses as 5 MiB, so any MB/KB/GB suffixed Size bound becomes much larger. A unit outside B/K/M/G throws instead of being read as bytes -- '1T' was a one byte bound that rejected every upload while reading as a generous one. blockExtensions() takes a required, non-empty list. allowAnyExtension() is the only way to empty it, so a config value that turns out to be missing cannot silently disable the control. upload() throws LogicException with no validations configured, getHash() throws InvalidArgumentException for an unsupported algorithm, and __call() throws BadMethodCallException for an unknown method. All were Upload\Exception -- the type isValid() formats into getErrors() -- so a typo in the caller's own source was reported to the end user as a rejected upload. isValid() re-throws LogicException from a validator for the same reason. 'File already exists' now names the file in the way, because sanitizing is many-to-one and the old wording could not say which name collided. FileInfoInterface's three setters no longer declare a return type. They declared : FileInfo, so an implementation that did not extend FileInfo satisfied the compiler and then raised a TypeError on the first setter call. A custom FileInfoInterface is only now actually implementable. getMd5() is gone and getHash() defaults to sha256. Verified by 373 tests on PHP 7.3 through 8.5, PHPStan level 9 over src and tests, and PSR-12. The branch was reviewed four times -- a seven-reviewer audit, a cleanup pass, an audit of every method this release adds, and a documentation pass -- each finding recorded in CHANGELOG.md. Fixes carry a test verified to fail against the code before them. Co-Authored-By: Claude Opus 5 (1M context) --- .github/README.md | 495 ++++++++++- .github/workflows/php-syntax.yml | 16 +- .github/workflows/phpcs.yml | 16 +- .github/workflows/phpstan.yml | 16 +- .github/workflows/phpunit.yml | 29 +- CHANGELOG.md | 102 ++- CLAUDE.md | 130 +++ LICENSE | 1 + UPGRADE.md | 246 ++++++ composer.json | 5 +- src/Upload/AsciiCase.php | 70 ++ src/Upload/Exception.php | 24 +- src/Upload/File.php | 404 +++++---- src/Upload/FileInfo.php | 277 +++--- src/Upload/FileInfoInterface.php | 30 +- src/Upload/Filename.php | 371 ++++++++ src/Upload/Storage/FileSystem.php | 510 ++++++++++- src/Upload/StorageInterface.php | 14 +- src/Upload/Validation/Extension.php | 31 +- src/Upload/Validation/FileType.php | 184 ++++ src/Upload/Validation/Mimetype.php | 19 +- src/Upload/Validation/Size.php | 33 +- src/Upload/ValidationInterface.php | 9 +- tests/Upload/ExceptionTest.php | 40 + tests/Upload/FileInfoTest.php | 269 +++++- tests/Upload/FileTest.php | 689 ++++++++++++++- tests/Upload/Storage/ExposedFileSystem.php | 20 + tests/Upload/Storage/FileSystemTest.php | 952 ++++++++++++++++++++- tests/Upload/Validation/FileTypeTest.php | 209 +++++ tests/Upload/Validation/SizeTest.php | 23 +- tests/bootstrap.php | 3 + 31 files changed, 4672 insertions(+), 565 deletions(-) create mode 100644 CLAUDE.md create mode 100644 UPGRADE.md create mode 100644 src/Upload/AsciiCase.php create mode 100644 src/Upload/Filename.php create mode 100644 src/Upload/Validation/FileType.php create mode 100644 tests/Upload/ExceptionTest.php create mode 100644 tests/Upload/Storage/ExposedFileSystem.php create mode 100644 tests/Upload/Validation/FileTypeTest.php diff --git a/.github/README.md b/.github/README.md index f7f4cc6..7ed626c 100644 --- a/.github/README.md +++ b/.github/README.md @@ -3,99 +3,502 @@ [![codecov](https://codecov.io/gh/GravityPDF/Upload/branch/main/graph/badge.svg)](https://codecov.io/gh/GravityPDF/Upload) [![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](https://opensource.org/licenses/MIT) -This component simplifies file validation and uploading. +A PHP library to validate and save uploaded files. **Why was this library forked?** * Original library was abandoned (untouched since 2018) -* Adjusted namespace from \Upload to \GravityPdf\Upload -* Bumped minimum PHP version to 7.3+ -* Sanitized filename and extension, and add UTF-8 filename support +* Safe defaults: existing files are never overwritten, executable and markup extensions are + never written, stored files get mode `0640`, and `upload()` requires validation +* `Validation\FileType` requires the extension and mimetype to match +* Sanitizes filenames and extensions, with UTF-8 filename support +* Stops path traversal, symlinked destinations, dotfiles and control characters in + filenames, and writes through a staged file moved into place rather than writing to the + destination directly * Strict type checking * Added `FileSystem::getDirectory()` and `FileInfo::setNameWithExtension()` methods * Included unreleased code from upstream repo +* Bumped minimum PHP version to 7.3+ * PSR-12 Code Formatting * Automated tools: PHPUnit, PHPStan, PHPCS, and PHP Syntax Checker -TODO: [PSR-7 and PSR-17 support (help wanted)](https://github.com/GravityPDF/Upload/issues/8) - ## Installation ``` composer require gravitypdf/upload ``` +### Requirements + +PHP 7.3 to 8.5 and `ext-fileinfo`. Optional but recommended: `ext-mbstring` (or `symfony/polyfill-mbstring`) so filenames can be guaranteed UTF-8. + +Migrating from `codeguy/upload`? Version 3.x of this package is a drop-in replacement: +update your imports from `\Upload\…` to `\GravityPdf\Upload\…`. + +Upgrading from 3.x? Version 4.0 turns new protections on by default. The +[upgrade guide](https://github.com/GravityPDF/Upload/blob/main/UPGRADE.md) covers what +changed and what to check. + ## Usage +### Single-file upload + Assume a file is uploaded with this HTML form: ```html
- +
``` -When the HTML form is submitted, the server-side PHP code can validate and upload the file like this: +Server-side, validate the upload, rename it, and store it: ```php -$storage = new \GravityPdf\Upload\Storage\FileSystem('/path/to/directory'); -// To override existing files when uploading, pass `true` as the second parameter -// $storage = new \GravityPdf\Upload\Storage\FileSystem('/path/to/directory', true); -$file = new \GravityPdf\Upload\File('foo', $storage); +use GravityPdf\Upload\File; +use GravityPdf\Upload\Storage\FileSystem; +use GravityPdf\Upload\Validation\FileType; +use GravityPdf\Upload\Validation\Size; -// Validate file upload -// MimeType List => http://www.iana.org/assignments/media-types/media-types.xhtml -$file->addValidations([ - // Ensure file is of type "image/png" - new \GravityPdf\Upload\Validation\Mimetype('image/png'), - new \GravityPdf\Upload\Validation\Extension('png'), +// Store uploads where the web server will not execute or serve them directly +$storage = new FileSystem('/path/to/uploads'); - //You can also add multi mimetype validation or extensions - //new \GravityPdf\Upload\Validation\Mimetype(['image/png', 'image/gif']) - //new \GravityPdf\Upload\Validation\Extension(['png', 'gif']), +// Reads $_FILES['avatar'] +$file = new File('avatar', $storage); - // Ensure file is no larger than 5M (use "B", "K", M", or "G") - new \GravityPdf\Upload\Validation\Size('5M'), +// upload() refuses to run unless at least one validation is added +$file->addValidations([ + new FileType('png', 'image/png'), // extension and file contents must both say PNG + new Size('2M'), // max 2 MiB ("B", "K", "M" or "G") ]); -// Access data about the file -// If upload accepts multiple files an array will be returned for each of these +// isValid() also checks is_uploaded_file(), so call it before reading any metadata +if ($file->isValid() === false) { + foreach ($file->getErrors() as $message) { + echo htmlspecialchars($message, ENT_QUOTES, 'UTF-8'), '
'; // always escape on output + } + + return; +} + +// Store under a random name; keep the client's (sanitized) name for display only +$displayName = $file->getNameWithExtension(); +$file->setName(bin2hex(random_bytes(16))); + +try { + $file->upload(); + + $storedPath = $file->getUploadedLocators()[0]; +} catch (\Exception $e) { + // Validation has already passed, so this is a storage failure: the destination + // exists, the extension is blocked, or the disk is full +} +``` + +`FileType` accepts any registered +[IANA media type](https://www.iana.org/assignments/media-types/media-types.xhtml), such as +`image/png` or `application/pdf`. + +### Reading file metadata + +```php $data = [ - 'name' => $file->getNameWithExtension(), - 'extension' => $file->getExtension(), - 'mime' => $file->getMimetype(), - 'size' => $file->getSize(), - 'md5' => $file->getMd5(), - 'dimensions' => $file->getDimensions(), + 'name' => $file->getNameWithExtension(), // sanitized client name; display only + 'extension' => $file->getExtension(), + 'mime' => $file->getMimetype(), // sniffed from the contents, not the client's claim + 'size' => $file->getSize(), // bytes, or false if the file is unreadable + 'hash' => $file->getHash(), // sha256 unless you pass another algorithm + 'dimensions' => $file->getDimensions(), // ['width' => int, 'height' => int] ]; +``` + +These calls are forwarded to every file in the collection. With one file you get the value +back, with several you get an array of values, and with none you get `null`. When a field +accepts multiple files, read metadata per file instead. + +### Multi-file upload + +```html +
+ + +
+``` + +The `$_FILES` key drops the brackets: `new File('photos', $storage)`. `File` acts as a +collection of the individual files, so `count()`, `foreach` and array offsets all work. A +file that failed to transfer (too large, nothing selected) is left out, and its error +message is already in `getErrors()`. -// If you have an upload field that accepts multiple files you can access each file's info individually -$firstFileName = $file[0]->getNameWithExtension(); -if(isset($file[1])) { - $secondFileName = $file[1]->getNameWithExtension(); +```php +use GravityPdf\Upload\File; +use GravityPdf\Upload\Storage\FileSystem; +use GravityPdf\Upload\Validation\FileType; +use GravityPdf\Upload\Validation\Size; + +$storage = new FileSystem('/path/to/uploads'); +$file = new File('photos', $storage); + +$file->addValidations([ + // One format per call, otherwise every extension is paired with every media type + (new FileType(['jpg', 'jpeg'], 'image/jpeg')) + ->allow('png', 'image/png') + ->allow('webp', 'image/webp'), + new Size('10M'), +]); + +// An empty collection has nothing to fail validation, so check the count as well +if (count($file) === 0 || $file->isValid() === false) { + foreach ($file->getErrors() as $message) { + echo htmlspecialchars($message, ENT_QUOTES, 'UTF-8'), '
'; + } + + return; } -// or loop over all files for this key -foreach($file as $i => $upload) { - $name = $upload->getNameWithExtension(); - $upload->setName('file-'.$i); +// Rename each file server-side, keeping the client names for display +$manifest = []; +foreach ($file as $photo) { + $displayName = $photo->getNameWithExtension(); + $photo->setName(bin2hex(random_bytes(16))); + + $manifest[] = [ + 'display' => $displayName, + 'stored' => $photo->getNameWithExtension(), + 'hash' => $photo->getHash(), + ]; } -// Try to upload file(s) try { - // Success! $file->upload(); } catch (\Exception $e) { - // Validation errors - $errors = $file->getErrors(); - if(count($errors) === 0) { - // Failed for another reason, like the file already exists - $error = $e->getMessage(); + // Multi-file uploads are not atomic: earlier files may already be on disk. + // getUploadedLocators() lists what was written so the batch can be rolled back. + // unlink() is right for Storage\FileSystem, which returns local paths. A storage + // of your own returns whatever locator it defines, so undo it the way it stores. + foreach ($file->getUploadedLocators() as $uploadedPath) { + unlink($uploadedPath); + } + + return; +} +``` + +Individual files are also reachable by offset. Check with `isset($file[0])` first, because +files that failed to transfer are missing from the collection. + +### Lifecycle callbacks + +Four optional hooks fire once per file, each receiving that file's `FileInfoInterface`: +`beforeValidate`, `afterValidate`, `beforeUpload` and `afterUpload`. Use them for per-file +work like renaming or audit logging without writing your own loops. + +The two validation hooks are a matched pair: `afterValidate` fires for every file +`beforeValidate` fired for, including one that failed, so they can safely open and close a +per-file resource. The upload hooks are not a pair — a storage failure throws out of +`upload()` before `afterUpload` runs. + +```php +use GravityPdf\Upload\FileInfoInterface; + +$file->beforeUpload(static function (FileInfoInterface $fileInfo): void { + $fileInfo->setName(bin2hex(random_bytes(16))); +}); + +$file->afterUpload(static function (FileInfoInterface $fileInfo): void { + error_log(sprintf('Stored upload as %s', $fileInfo->getNameWithExtension())); +}); +``` + +**`beforeUpload` runs after validation, not before it**, so a name set there is never +validated — only the storage deny-list and `FileSystem`'s filename rules apply. `setName()` +is safe there, since it cannot change the extension. `setExtension()` and +`setNameWithExtension()` are not: given anything derived from user input, they can store a +file under an extension your validations would have rejected, and the deny-list covers only +[the formats below](#extensions-blocked-by-default). + +Rename in `beforeValidate` if the final name has to be the validated one. + +### Custom validation rules + +Implement `ValidationInterface` and throw `GravityPdf\Upload\Exception` to reject a file. +The exception message is what `getErrors()` shows the end user: + +```php +use GravityPdf\Upload\Exception; +use GravityPdf\Upload\FileInfoInterface; +use GravityPdf\Upload\ValidationInterface; + +class MaxDimensions implements ValidationInterface +{ + private $maxWidth; + private $maxHeight; + + public function __construct(int $maxWidth, int $maxHeight) + { + $this->maxWidth = $maxWidth; + $this->maxHeight = $maxHeight; + } + + public function validate(FileInfoInterface $fileInfo): void + { + $size = $fileInfo->getDimensions(); + + if ($size['width'] > $this->maxWidth || $size['height'] > $this->maxHeight) { + throw new Exception( + sprintf('Image must be no larger than %dx%d pixels', $this->maxWidth, $this->maxHeight), + $fileInfo + ); + } + } +} + +$file->addValidation(new MaxDimensions(2048, 2048)); +``` + +Failures accumulate rather than abort: every validation runs against every file, and +`getErrors()` reports them all at once. + +Throwing anything other than `GravityPdf\Upload\Exception` is caught too, but nothing it +carries reaches `getErrors()` — not its message, which can leak server paths, and not its +class name. Catch it in the validator and rethrow an `Upload\Exception` if either belongs in +what the user sees. + +`\LogicException` is the exception to that: it propagates out of `isValid()`, since PHP +defines the type as a bug in your program rather than a file that failed. A validator +calling `getHash()` with a misspelled algorithm reaches you, not the end user. + +### Custom storage backends + +Implement `StorageInterface` to store files somewhere other than the local filesystem: +read from `$fileInfo->getPathname()`, return the destination, and throw +`GravityPdf\Upload\Exception` on failure. + +The string you return is a locator you define — a key, a URL, an identifier — +and `File::getUploadedLocators()` hands it back unchanged, so it is what the application +rolls back with. Never return `''`: a caller cannot tell it from a usable value. + +```php +use GravityPdf\Upload\FileInfoInterface; +use GravityPdf\Upload\StorageInterface; + +class ObjectStorage implements StorageInterface +{ + public function upload(FileInfoInterface $fileInfo): string + { + $key = 'uploads/' . $fileInfo->getNameWithExtension(); + + // ... stream $fileInfo->getPathname() to your object store ... + + return $key; } } ``` +The protections under "Security notes" — the deny-list, the `basename()` reduction, the +symlink refusal, the staged write — live in `Storage\FileSystem`. A custom backend needs +its own equivalents. + +## Security notes + +**Prefer `FileType` over `Mimetype` and `Extension` separately.** Those two check independent +lists, so content sniffed as `image/gif` stored as `avatar.png` satisfies both. `FileType` +requires the extension and the contents to describe the same format. + +**Generate storage names server-side.** Sanitizing normalizes client names, so `report.txt` +and `report!.txt` both land on `report.txt`. A server-side name avoids the collision and the +predictable destination: + +```php +$file->setName(bin2hex(random_bytes(16))); // keep the client name as display metadata only +``` + +**Sanitizing is not escaping.** Unsafe characters are rewritten, not escaped. Escape on +output and use parameterized queries. This applies to `getErrors()`. + +**Show `getErrors()`, log the exception.** Every string in `getErrors()` is sanitized and +describes the submitted file. A storage exception message is not: it distinguishes a name +that already exists from a destination that could not be created, which tells whoever +submitted the file what is in your upload directory. Log it and show something generic. + +**Call `isValid()` before reading metadata.** It performs the `is_uploaded_file()` check; +the metadata accessors do not. This matters where `$_FILES` is rebuilt by something other +than the PHP SAPI (PSR-7 bridges, test harnesses, middleware). + +**Serve uploads from a directory the web server won't execute.** The storage defaults below +are backstops, not a substitute for that. + +### Extensions blocked by default + +`FileSystem` refuses to write these, whatever the validations allowed. The check runs against +the sanitized extension about to be written and throws `\GravityPdf\Upload\Exception` rather +than recording a validation error. + +Dots inside the name are not extension separators: `FileInfo::setName()` rewrites them to +hyphens, so `release.config.zip` is stored as `release-config.zip`. `upload()` checks every +dot-separated component regardless, for names reaching it from a `FileInfoInterface` of +your own. + +| Group | Extensions | +|---|---| +| PHP | `php` `php2` `php3` `php4` `php5` `php6` `php7` `php8` `phps` `phtml` `phtm` `phar` `pht` `inc` | +| Server-side includes | `shtml` `shtm` `stm` | +| CGI and scripts | `cgi` `fcgi` `pl` `py` `rb` `sh` `bash` `ps1` | +| Java | `jsp` `jspx` `jspf` `jsw` `jsv` `jshtml` `jar` `war` | +| ASP / ASP.NET | `asp` `aspx` `asa` `asax` `ascx` `ashx` `asmx` `cer` `cshtml` `vbhtml` | +| Windows binaries | `exe` `dll` `com` `bat` `cmd` `msi` `scr` `vbs` `ws` `wsf` `hta` | +| Server configuration | `htaccess` `htpasswd` `ini` `conf` `config` | +| Markup and script | `html` `htm` `xhtml` `xht` `xhtm` `svg` `svgz` `xml` `xsl` `xslt` `js` `mjs` `swf` `mht` `mhtml` | + +The first seven groups are `FileSystem::EXECUTABLE_EXTENSIONS`, which a server runs. The +last is `FileSystem::MARKUP_EXTENSIONS`, which a browser renders — serving one from your own +origin is stored XSS. SVG is in that group because it carries `