Path Traversal in MediaUploadTrait::deleteFile() Allows Arbitrary File Deletion

Summary

A path traversal vulnerability in MediaUploadTrait::deleteFile() allows an authenticated user with media management permissions to delete arbitrary files on the server. The method validates only the basename portion of the filename using Utils::checkFilename(), while the directory path (which may contain ../ sequences) is preserved and passed unvalidated to unlink(). This enables directory escape from the intended media storage path.

Severity

High (8.1) - CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:H

CWE

CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')

Details

In system/src/Grav/Common/Media/Traits/MediaUploadTrait.php, the deleteFile() method (lines 332-365) performs filename validation only on the basename, not the full path:

public function deleteFile(string $filename, ?array $settings = null): void
{
    $settings = $this->getUploadSettings($settings);
    $filesystem = Filesystem::getInstance(false);

    // Line 339-340: Only the BASENAME is validated
    $basename = $filesystem->basename($filename);  // e.g. "evil.jpg" from "../../evil.jpg"
    if (!Utils::checkFilename($basename)) {         // passes - no traversal in basename
        throw new RuntimeException(/* ... */);
    }

    $path = $settings['destination'] ?? $this->getPath();
    // ...

    // Line 353: Full pathname (with traversal) is preserved
    $pathname = $filesystem->pathname($filename);   // "../../"

    // Line 356-357: Traversal path reconstructed
    [$base, $ext,,] = $this->getFileParts($basename);
    $name = "{$pathname}{$base}.{$ext}";            // "../../evil.jpg"

    // Line 360: Passed to doRemove()
    $this->doRemove($name, $path);
}

doRemove() (line 521-582) then calls:

// Line 538
unlink("{$folder}/{$filename}");
// e.g. unlink("/var/www/grav/user/pages/mypage/../../config/system.yaml")

Utils::checkFilename() (lines 1022-1044) properly checks for /, \, and .., but it is applied to $filesystem->basename($filename) (the last path component only), so traversal sequences in the directory portion are never validated.

Data flow from user input

The vulnerability is reachable through the Flex media handling pipeline:

1. FlexMediaTrait::setUpdatedMedia() (line 386) iterates form flash data where $filename is the array key - user-controlled
2. For file deletions ($file is null, line 396), NO upload validation is performed (the checkUploadedFile() call at line 401 only executes when $file is truthy)
3. The raw filename is stored in $this->_uploads at line 414
4. saveUpdatedMedia() (line 499) calls $media->deleteFile($filename, $settings) with the unsanitized filename

Sibling: renameFile()

The same pattern exists in renameFile() (lines 374-405) which has even weaker validation - it performs NO checkFilename() call at all. While renameFile() currently has no callers in the core codebase, it is part of the public MediaUploadInterface and should be fixed as defense-in-depth.

Proof of Concept

Environment: Grav CMS 2.0.16 with admin plugin

The attack requires an authenticated admin user with page/media editing permissions (not super-admin).

1. Create a target file:

echo "DELETE_ME" > /var/www/grav/user/data/target.txt

2. Submit a Flex object form (e.g. page edit) with a crafted media deletion where the filename key contains path traversal:

POST /admin/pages/mypage/task:save
Content-Type: multipart/form-data

# The form flash data includes a media deletion entry with key:
# "../../data/target.txt" -> null (deletion marker)

3. When saveUpdatedMedia() processes the deletion queue:

   • $filename = ../../data/target.txt
   • deleteFile("../../data/target.txt") is called
   • $basename = target.txt (passes checkFilename())
   • $pathname = ../../data/
   • $name = ../../data/target.txt
   • doRemove() calls unlink("/var/www/grav/user/pages/mypage/../../data/target.txt")
   • Which resolves to unlink("/var/www/grav/user/data/target.txt")

4. The file is deleted outside the intended media directory.

Impact

An authenticated user with media management permissions can:

• Delete configuration files (user/config/system.yaml, user/config/security.yaml)
• Delete other pages' content files
• Delete authentication-related files (user account YAML files)
• Cause denial of service by removing critical application files
• Potentially escalate privileges by removing security configuration

Suggested Fix

Apply Utils::checkFilename() to the full $filename parameter before decomposing it, or reject any filename containing directory separators or .. sequences:

public function deleteFile(string $filename, ?array $settings = null): void
{
    $settings = $this->getUploadSettings($settings);
    $filesystem = Filesystem::getInstance(false);

    // Validate the FULL filename, not just the basename
    if (!Utils::checkFilename($filename)) {
        throw new RuntimeException(/* ... */);
    }

    // ... rest unchanged
}

The same fix should be applied to renameFile() for both $from and $to parameters.

References

• Vulnerable file: system/src/Grav/Common/Media/Traits/MediaUploadTrait.php lines 332-365, 521-582
• Caller: system/src/Grav/Framework/Flex/Traits/FlexMediaTrait.php lines 386-414, 490-499
• Sibling: system/src/Grav/Common/Media/Traits/MediaUploadTrait.php lines 374-405 (renameFile)
• Related GHSA: GHSA-g6j3-8jv9-ch5f (path traversal in PagesController::batchCopy - different file, same bug class)

Disclosure

This vulnerability was discovered using AI-assisted security research tools.