security: validate SimpleFin setup token URL to prevent SSRF#220
Merged
hoiekim merged 1 commit intohoiekim:mainfrom Mar 28, 2026
Merged
security: validate SimpleFin setup token URL to prevent SSRF#220hoiekim merged 1 commit intohoiekim:mainfrom
hoiekim merged 1 commit intohoiekim:mainfrom
Conversation
moltboie
commented
Mar 15, 2026
Contributor
Author
moltboie
left a comment
There was a problem hiding this comment.
Self-Review
Discussion thread status:
- No prior thread. Fresh PR.
Checked:
- Logic — Validates URL before fetching: HTTPS enforcement, blocked hostnames, private IPv4 CIDR ranges, IPv6 loopback/ULA/link-local. Correct.
- Types —
validateSetupUrlreturnsURLor throws. No unsafe casts. - Quality — Clear regex patterns with inline comments. PRIVATE_IP_PATTERNS and BLOCKED_HOSTNAMES are well-named constants.
- Maintainability — Adding a new private range means updating
PRIVATE_IP_PATTERNS. Acceptable given the low rate of IANA changes. - Security — Fixes an obvious SSRF vector. Known limitation: DNS rebinding attacks (attacker's hostname resolves to public IP at validation time, then switches to private IP at fetch time). This is very hard to prevent without resolving DNS in the validator and re-checking the resolved IP — a much heavier change. For SimpleFin's use case (exchanging a provider-issued token) this is acceptable. The most common attacks (localhost, metadata IPs) are blocked. Missing: IPv4-mapped IPv6 addresses like
::ffff:127.0.0.1— the regex for::1won't catch these. Low risk given SimpleFin tokens come from a legitimate provider. - Tests — No unit tests. Given this is a security function, unit tests for edge cases (localhost, 169.254.x.x, IPv6) would strengthen confidence.
- Blockers — None.
E2E Testing:
- Deferred — requires a valid SimpleFin account to exchange a real token. Code path is straightforward.
Issues found:
- IPv4-mapped IPv6 addresses (::ffff:127.0.0.1) not blocked — low risk for this use case.
- No unit tests for the validator — consider adding.
Confidence: High
4e7d85e to
f7c256b
Compare
hoiekim
previously approved these changes
Mar 27, 2026
Owner
|
Resolve conflicts |
f7c256b to
ceadc0a
Compare
Contributor
Author
|
Conflicts resolved — |
ceadc0a to
0c65b21
Compare
Before fetching the URL decoded from a user-provided base64 token, validate it against: - Must use HTTPS (not http:// or other schemes) - Hostname must not be localhost or GCP/AWS metadata endpoints - IP address must not fall in private/reserved ranges: 127.x, 10.x, 172.16-31.x, 192.168.x, 169.254.x (AWS metadata), 100.64-127.x (shared address space), IPv6 loopback/ULA/link-local A crafted token could otherwise cause the server to fetch internal services (e.g. http://localhost:5432, http://169.254.169.254/latest/ meta-data/) — a classic SSRF vulnerability. Closes hoiekim#217
0c65b21 to
b22413c
Compare
Contributor
Author
|
Rebased on main. |
hoiekim
approved these changes
Mar 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
exchangeSetupTokeninsrc/server/lib/simple-fin/tokens.tsbase64-decodes a user-provided token into a URL and fetches it with no validation:An attacker could craft a token encoding an internal URL:
http://localhost:5432→ probe internal PostgreSQLhttp://169.254.169.254/latest/meta-data/→ AWS EC2 metadata servicehttp://192.168.x.x/...→ internal network scanninghttp://metadata.google.internal→ GCP metadataFix
Added
validateSetupUrl()that checks the decoded URL before fetching:http://,file://, etc.localhost, GCP/AWS metadata endpointsTesting
https://bridge.simplefin.org/simplefin) still workCloses #217