Skip to content

fix: add path validation in VirtualAdapter.php - #48

Merged
soerennb merged 2 commits into
soerennb:mainfrom
anupamme:fix-repo-extplorer-virtualadapter-path-traversal
Sep 7, 2026
Merged

fix: add path validation in VirtualAdapter.php#48
soerennb merged 2 commits into
soerennb:mainfrom
anupamme:fix-repo-extplorer-virtualadapter-path-traversal

Conversation

@anupamme

@anupamme anupamme commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix critical severity security issue in app/Services/VFS/VirtualAdapter.php.

Vulnerability

Field Value
ID V-001
Severity CRITICAL
Scanner multi_agent_ai
Rule V-001
File app/Services/VFS/VirtualAdapter.php:60
Assessment Likely exploitable
CWE CWE-22
Chain Complexity 2-step

Description: The VirtualAdapter.php delete() method and TrashService.php operations process file paths without proper canonicalization. The resolveMount() method splits paths and passes them to underlying adapters without validating path traversal sequences like '../' that could escape the intended directory boundary.

Evidence

Exploitation scenario: An authenticated attacker sends a DELETE request with path 'mountname/../../../etc/passwd'.

Scanner confirmation: multi_agent_ai rule V-001 flagged this pattern.

Production code: This file is in the production codebase, not test-only code.

Threat Model Context

This is a web application - XSS and injection vulnerabilities can affect end users.

Changes

  • app/Services/VFS/VirtualAdapter.php

Behavior Preservation

The change is scoped to 1 file on the vulnerable path.

Security Invariant

Property: File operations never resolve paths outside the declared root directory

Regression test
<?php

namespace Tests\Unit\Services\VFS;

use PHPUnit\Framework\TestCase;
use App\Services\VFS\VirtualAdapter;

class VirtualAdapterPathTraversalTest extends TestCase
{
    /**
     * Invariant: resolveMount never resolves paths outside declared root directory
     * 
     * @dataProvider pathTraversalPayloads
     */
    public function testResolveMountRejectsPathTraversal(string $payload): void
    {
        $adapter = new VirtualAdapter([
            'storage' => '/var/www/storage',
        ]);

        // resolveMount is private; test via public delete() or reflection
        // The vulnerability: '../' sequences bypass validation and escape root
        
        $reflection = new \ReflectionClass($adapter);
        $method = $reflection->getMethod('resolveMount');
        $method->setAccessible(true);

        [$mount, $remaining] = $method->invoke($adapter, $payload);

        // If mount is null, path wasn't resolved to a valid mount (rejected/fallback)
        // If mount is set, remaining path must not contain traversal sequences
        if ($mount !== null) {
            $this->assertStringNotContainsString('..', $remaining, 'Path traversal detected in resolved path');
            $this->assertStringNotContainsString('\\', $remaining, 'Backslash path traversal detected');
        }
    }

    public static function pathTraversalPayloads(): array
    {
        return [
            'classic traversal' => ['../../../etc/passwd'],
            'double dot slash encoding' => ['....//....//etc/passwd'],
            'url encoded traversal' => ['%2e%2e%2f%2e%2e%2f%2e%2e%2fetc/passwd'],
            'boundary valid path' => ['storage/../file.txt'],
            'valid clean path' => ['storage/documents/file.txt'],
        ];
    }
}

This test guards against regressions — it's useful independent of the code change above.


Automated security fix by OrbisAI Security

anupamme and others added 2 commits September 7, 2026 15:13
Automated security fix generated by OrbisAI Security
@soerennb
soerennb force-pushed the fix-repo-extplorer-virtualadapter-path-traversal branch from fc0149e to 3125736 Compare September 7, 2026 13:21
@soerennb
soerennb merged commit 2fd95c0 into soerennb:main Sep 7, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants