Repository navigation
[GHSA-cxf4-7mrp-vvpr] Flysystem: WhitespacePathNormalizer's control-character (CorruptedPathDetected) check is bypassed by malformed UTF-8 in the path, affecting every adapter - #9858
Conversation
|
Hi there @frankdejonge! A community member has suggested an improvement to your security advisory. If approved, this change will affect the global advisory listed at github.com/advisories. It will not affect the version listed in your project repository. This change will be reviewed by our Security Curation Team. If you have thoughts or feedback, please share them in a comment here! If this PR has already been closed, you can start a new community contribution for this advisory |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The impact analysis is technically sound, with only a minor grammatical correction noted.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds an impact analysis explaining how malformed UTF-8 paths can affect downstream consumers.
Changes:
- Documents spoofing, injection, tooling, and encoding risks.
- Clarifies prerequisites and adapter limitations.
| File | Description |
|---|---|
GHSA-cxf4-7mrp-vvpr.json |
Adds the advisory’s impact section. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ], | ||
| "summary": "Flysystem: WhitespacePathNormalizer's control-character (CorruptedPathDetected) check is bypassed by malformed UTF-8 in the path, affecting every adapter", | ||
| "details": "## Related public issue (context, not a duplicate)\n\nClosed issue #1429 (\"Handle non-UTF-8 paths\", 2024-03-24) raised exactly this general concern and\neven suggested detection via `preg_match('//u', $path) !== 1` -- note the reporter's suggested check\nexplicitly compares `!== 1`, which *would* correctly treat PCRE's `false` return as \"reject.\" The\nmaintainer's reply pointed to the `PathNormalizer` interface as the place to implement this. The\ncontrol-character check that ended up shipping in `WhitespacePathNormalizer`\n(`if (preg_match('#\\p{C}+#u', $unixPath))`) addresses the general concern but does **not** use the\n`!== 1`-style comparison the original issue suggested -- it uses a bare truthy check, which is exactly\nthe gap this report demonstrates. So this is not a duplicate of #1429; it's a concrete bypass\nsurviving in the fix that issue's concern led to.\n\n## Vulnerability Details\n\n**File**: `src/WhitespacePathNormalizer.php`, lines 22-28 (`normalizePath()`) -- the **default**\n`PathNormalizer` used by `Filesystem` for every adapter (Local, FTP, SFTP, S3, AsyncAwsS3, Azure, GCS,\nZipArchive, GridFS, InMemory) unless the application supplies a custom one.\n\n### Root Cause\n\n```php\npublic function normalizePath(string $path): string\n{\n $unixPath = str_replace('\\\\', '/', $path);\n\n if (preg_match('#\\p{C}+#u', $unixPath)) {\n throw CorruptedPathDetected::forPath($path);\n }\n ...\n```\n\n`preg_match()` returns `false` (a PHP engine error) rather than `0` when the subject string is not\nvalid UTF-8 and the pattern uses the `/u` modifier -- PCRE can't even attempt the match. `false` and\n`0` are both falsy in PHP, and `if (preg_match(...))` does not distinguish them. So a path containing\n**any single invalid UTF-8 byte anywhere in the string** makes `preg_match()` fail with a \"Malformed\nUTF-8 characters\" engine error, the `if` evaluates false, and `CorruptedPathDetected` is silently **not**\nthrown -- even when the same string also contains literal control characters this exact check exists\nto catch.\n\nthe identical payload IS correctly rejected once it's valid UTF-8:\n```php\n$n->normalizePath(\"foo\\x1bbar\"); // valid UTF-8, contains ESC -> throws CorruptedPathDetected (correct)\n$n->normalizePath(\"foo\\x80\\x1bbar\"); // 0x80 = invalid lone UTF-8 continuation byte -> NOT thrown (bypass)\n```\n\nThe path-traversal protection is **unaffected** -- it's exact byte-string comparison on\n`/`-delimited segments, independent of UTF-8 validity:\n```php\n$n->normalizePath(\"\\x80/../../etc/passwd\"); // still throws PathTraversalDetected\n```\n\n### Recommended Fix\n```php\npublic function normalizePath(string $path): string\n{\n $unixPath = str_replace('\\\\', '/', $path);\n $matched = preg_match('#\\p{C}+#u', $unixPath);\n\n if ($matched !== 0) {\n // $matched === false means malformed UTF-8 -- must also be treated as corrupted,\n // not silently allowed through.\n throw CorruptedPathDetected::forPath($path);\n }\n ...\n```\n\n### Verification\nDynamically confirmed on `league/flysystem` HEAD `6837e1d` / tag `3.35.2`, PHP 8.4.22 CLI, end-to-end\nthrough `LocalFilesystemAdapter`:\n```\n[1] write() succeeded -- normalizer did NOT reject the path.\n[2] Actual bytes on disk: ...801b5b386d6e6f726d616c2d6c6f6f6b696e672d66696c652e7478741b5b306d...\n[3] Filesystem::listContents() path contains raw ESC (0x1b): YES\n[4] Raw terminal output (via `cat -v`): M-^@^[[8mnormal-looking-file.txt^[[0m^[[2K^[[1Aurgent-invoice.pdf", | ||
| "details": "## Impact\n\nWhen a path contains any byte that is not valid UTF-8, the CorruptedPathDetected check is skipped entirely. Any control or format character (Unicode category `\\p{C}`) can then reach storage, including `ESC`, `CR/LF`, and bidirectional overrides such as `U+202E` (Right-To-Left Override). Path traversal protection is not affected.\n\nThe impact comes from downstream consumers that trust the stored path:\n* **Display spoofing**: bidirectional overrides and zero-width characters could make a filename look like something else in web UIs or file listings (e.g. an executable that appears to be `.pdf`)\n* **Terminal/log injection**: ANSI escape sequences or CR/LF in filenames could hide, rewrite or forge output when an user views listings or logs in a terminal. On some emulators this could extend to clipboard writes (OSC 52)\n* **Line-based tooling**: embedded newlines could make shell scripts and cron jobs that process `ls`/`find` output act on unintended paths\n* **Encoding failures**: invalid UTF-8 in stored names could make `json_encode()` fail, cause database rejection or truncation, or break XML-based consumers, disrupting listings or desynchronising metadata from stored files.\n\nExploitation requires an attacker to control a path passed to Flysystem without prior sanitisation. Applications that generate their own storage names, or sanitise user-supplied filenames first, are not affected in practice. Whether the path can be written depends on the adapter. Local filesystems on POSIX systems accept it; some object stores and Windows filesystems may reject it.\n\n## Related public issue (context, not a duplicate)\n\nClosed issue #1429 (\"Handle non-UTF-8 paths\", 2024-03-24) raised exactly this general concern and\neven suggested detection via `preg_match('//u', $path) !== 1` -- note the reporter's suggested check\nexplicitly compares `!== 1`, which *would* correctly treat PCRE's `false` return as \"reject.\" The\nmaintainer's reply pointed to the `PathNormalizer` interface as the place to implement this. The\ncontrol-character check that ended up shipping in `WhitespacePathNormalizer`\n(`if (preg_match('#\\p{C}+#u', $unixPath))`) addresses the general concern but does **not** use the\n`!== 1`-style comparison the original issue suggested -- it uses a bare truthy check, which is exactly\nthe gap this report demonstrates. So this is not a duplicate of #1429; it's a concrete bypass\nsurviving in the fix that issue's concern led to.\n\n## Vulnerability Details\n\n**File**: `src/WhitespacePathNormalizer.php`, lines 22-28 (`normalizePath()`) -- the **default**\n`PathNormalizer` used by `Filesystem` for every adapter (Local, FTP, SFTP, S3, AsyncAwsS3, Azure, GCS,\nZipArchive, GridFS, InMemory) unless the application supplies a custom one.\n\n### Root Cause\n\n```php\npublic function normalizePath(string $path): string\n{\n $unixPath = str_replace('\\\\', '/', $path);\n\n if (preg_match('#\\p{C}+#u', $unixPath)) {\n throw CorruptedPathDetected::forPath($path);\n }\n ...\n```\n\n`preg_match()` returns `false` (a PHP engine error) rather than `0` when the subject string is not\nvalid UTF-8 and the pattern uses the `/u` modifier -- PCRE can't even attempt the match. `false` and\n`0` are both falsy in PHP, and `if (preg_match(...))` does not distinguish them. So a path containing\n**any single invalid UTF-8 byte anywhere in the string** makes `preg_match()` fail with a \"Malformed\nUTF-8 characters\" engine error, the `if` evaluates false, and `CorruptedPathDetected` is silently **not**\nthrown -- even when the same string also contains literal control characters this exact check exists\nto catch.\n\nthe identical payload IS correctly rejected once it's valid UTF-8:\n```php\n$n->normalizePath(\"foo\\x1bbar\"); // valid UTF-8, contains ESC -> throws CorruptedPathDetected (correct)\n$n->normalizePath(\"foo\\x80\\x1bbar\"); // 0x80 = invalid lone UTF-8 continuation byte -> NOT thrown (bypass)\n```\n\nThe path-traversal protection is **unaffected** -- it's exact byte-string comparison on\n`/`-delimited segments, independent of UTF-8 validity:\n```php\n$n->normalizePath(\"\\x80/../../etc/passwd\"); // still throws PathTraversalDetected\n```\n\n### Recommended Fix\n```php\npublic function normalizePath(string $path): string\n{\n $unixPath = str_replace('\\\\', '/', $path);\n $matched = preg_match('#\\p{C}+#u', $unixPath);\n\n if ($matched !== 0) {\n // $matched === false means malformed UTF-8 -- must also be treated as corrupted,\n // not silently allowed through.\n throw CorruptedPathDetected::forPath($path);\n }\n ...\n```\n\n### Verification\nDynamically confirmed on `league/flysystem` HEAD `6837e1d` / tag `3.35.2`, PHP 8.4.22 CLI, end-to-end\nthrough `LocalFilesystemAdapter`:\n```\n[1] write() succeeded -- normalizer did NOT reject the path.\n[2] Actual bytes on disk: ...801b5b386d6e6f726d616c2d6c6f6f6b696e672d66696c652e7478741b5b306d...\n[3] Filesystem::listContents() path contains raw ESC (0x1b): YES\n[4] Raw terminal output (via `cat -v`): M-^@^[[8mnormal-looking-file.txt^[[0m^[[2K^[[1Aurgent-invoice.pdf", |
ce9a1d4
into
Unifex/advisory-improvement-9858
|
Hi @Unifex! Thank you so much for contributing to the GitHub Advisory Database. This database is free, open, and accessible to all, and it's people like you who make it great. Thanks for choosing to help others. We hope you send in more contributions in the future! |

Updates
Comments
While the Description described what the issue was, and the fix, it did not describe the potential impact. This is a best effort understanding and is not my strong suit. If someone else could check this is correct that would be helpful.