Skip to content

fix: prevent SSRF in URL upload by blocking private/internal IPs#276

Open
tranquac wants to merge 1 commit intoandreimarcu:masterfrom
tranquac:fix/ssrf-url-upload
Open

fix: prevent SSRF in URL upload by blocking private/internal IPs#276
tranquac wants to merge 1 commit intoandreimarcu:masterfrom
tranquac:fix/ssrf-url-upload

Conversation

@tranquac
Copy link
Copy Markdown

Summary

Prevent SSRF in the URL upload handler by validating that the target URL doesn't resolve to private or internal IP addresses.

Problem

The URL upload handler fetches user-supplied URLs without any SSRF protection:

grabUrl, _ := url.Parse(r.FormValue("url"))
resp, err := http.Get(grabUrl.String())

An attacker can use this to:

  • Access cloud metadata: http://169.254.169.254/latest/meta-data/ → leaks IAM credentials
  • Scan internal network services
  • Access internal admin panels

Fix

  1. Validate URL scheme (only http/https)
  2. Resolve hostname and check all IPs against Go's IsLoopback(), IsPrivate(), IsLinkLocalUnicast()
  3. Handle URL parse errors (the original code discarded the error with _)

Impact

  • Type: Server-Side Request Forgery (CWE-918)
  • Affected endpoint: URL upload handler
  • Risk: Cloud credential theft, internal network scanning
  • OWASP: A10:2021 — Server-Side Request Forgery

Signed-off-by: tranquac <tranquac@users.noreply.github.com>
@mutantmonkey
Copy link
Copy Markdown
Contributor

This does not prevent SSRF in all cases; there are still plenty of things you can do if you can make arbitrary web requests to non-internal URLs. If you wish to prevent SSRF, you should disable the remote uploads feature; it is intentionally disabled by default. Perhaps the risks of enabling this feature should be better documented.

Additionally, please review the readme of this repository. I suggest you submit your changes to one of the forks.

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