Skip to content

BAU-27505 Fix Server-Side Request Forgery (SSRF) / Path Traversal issues #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

remus-iesan-natterbox
Copy link

Server-Side Request Forgery (SSRF) / Path Traversal
The fix involved adding strict validation to the getURL method in the S3 stream wrapper to enhance security. The changes ensure that only valid s3:// URLs, bucket names, and object paths are accepted, effectively preventing path traversal and SSRF attacks. These security checks do not alter existing functionality for valid inputs.
Following our changes, the vulnerabilities flagged by automated scanners (Path Traversal and SSRF) are considered false positives, as all user input is now validated before being used in sensitive operations. No exploitation is possible with the current implementation.

@natterbox-integration
Copy link

🎉 Snyk checks have passed. No issues have been found so far.

code/snyk check is complete. No issues have been found. (View Details)

Copy link

@JamesHayes-NB JamesHayes-NB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSRF Mitigation:

  • The code now enforces that only the s3:// scheme is allowed. If the scheme is missing or not s3, it throws an InvalidArgumentException. This prevents attackers from using other URI schemes (like http, file, etc.) to trigger SSRF.
    if (!isset($this->url['scheme']) || $this->url['scheme'] !== 's3') {
        throw new InvalidArgumentException('Invalid scheme: only s3:// is allowed');
    }

Bucket Name Validation:

  • The bucket name (host part of the URL) is validated against a regex to ensure it conforms to S3 bucket naming rules and is not an IP address or malformed. This further prevents SSRF by disallowing URLs that could point to internal resources.
    if (
        !isset($this->url['host']) ||
        !preg_match('/^(?!^\d+\.\d+\.\d+\.\d+$)(?!.*\.\.)(?!.*\.$)(?!^\.)[a-z0-9]([a-z0-9.-]{1,61}[a-z0-9])?$/', $this->url['host'])
    ) {
        throw new InvalidArgumentException('Invalid S3 bucket name');
    }

Path Traversal Mitigation:

  • The path component is sanitized (leading slashes are removed), and the code checks if '..' appears in the path. If so, it throws an exception, blocking directory traversal attacks.
    $this->url['path'] = isset($this->url['path']) ? ltrim($this->url['path'], '/') : '';
    if (strpos($this->url['path'], '..') !== false) {
        throw new InvalidArgumentException('Invalid S3 object path');
    }

Summary:
These changes add strong validation and error handling to prevent SSRF and path traversal exploits by restricting schemes, validating bucket names, and ensuring object paths cannot be abused. This is a good security improvement.

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.

3 participants