T09 · Insecure Skill Coding Practices
Error
- Location
- references/class-examples.md:691
- Finding
- Untrusted Upload Filenames Allow Path Traversal and Arbitrary File Writes<![CDATA[ ## Vulnerability Details **File Location**: `references/class-examples.md:691-700` and `references/class-examples.md:716-723` **Vulnerability Type**: Unrestricted file path construction from attacker-controlled upload filenames **Risk Level**: High ### Vulnerable Code ```php class UploadHandler implements RequestHandler { public function handleRequest(Request $request): Response { $form = Form::fromRequest($request); $username = $form->getValue('username'); $file = $form->getFile('avatar'); if ($file !== null) { \Amp\File\write('/uploads/' . $file->getFilename(), $file->getContents()); } return new Response(HttpStatus::OK, [], "Hello, $username"); } } ``` ```php class StreamingUploadHandler implements RequestHandler { public function handleRequest(Request $request): Response { $parser = new StreamingFormParser(); $parser->parseForm($request, function (mixed $field) { if ($field instanceof FileField) { $dest = File\openFile('/uploads/' . $field->getFilename(), 'w'); ByteStream\pipe($field->getStream(), $dest); } }); return new Response(HttpStatus::OK, [], 'Uploaded'); } } ``` ### Technical Analysis Both examples use the client-supplied multipart filename directly when constructing a destination path. Multipart filenames are attacker-controlled and may contain traversal sequences such as `../`, platform-specific separators, absolute path components, or names of existing files. Concatenating such a filename with `/uploads/` does not guarantee that the resulting normalized path remains inside the intended upload directory. Opening the destination with write mode may also truncate an existing file. The examples do not: - Remove directory components from the supplied filename. - Generate a server-controlled filename. - Resolve and validate the canonical destination path. ...[truncated 1683 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Do not use the client-provided filename as the storage filename. - Generate a cryptographically random server-side name, for example with `bin2hex(random_bytes(16))`. - If retaining part of the original filename is necessary, first apply `basename()`, normalize platform separators, and enforce a strict allowlist. - Resolve the canonical upload root and destination parent, then verify that the resulting path remains within the upload root. - Store uploads outside the document root whenever possible. - Prevent overwrites by using exclusive creation semantics or rejecting destinations that already exist. - Enforce file-size, MIME-type, extension, and content validation appropriate to the application. - Configure the upload directory as non-executable. - Run the PHP process with filesystem permissions limited to the specific upload directory. A safer storage pattern is: ```php $uploadRoot = '/uploads'; $extension = validatedExtension($field); $filename = bin2hex(random_bytes(16)) . '.' . $extension; $destination = $uploadRoot . DIRECTORY_SEPARATOR . $filename; $dest = File\openFile($destination, 'x'); ByteStream\pipe($field->getStream(), $dest); ``` The implementation should additionally verify that `$uploadRoot` is the intended canonical directory and that `validatedExtension()` uses a strict application-specific allowlist. ]]>