fix: validate location input to prevent SSRF via URL fragment injection#328
fix: validate location input to prevent SSRF via URL fragment injection#328adilburaksen wants to merge 1 commit into
Conversation
…URL fragment injection The `location` field parsed from user-supplied secret references was interpolated directly into the Secret Manager API endpoint URL without any format validation: https://secretmanager.${ref.location}.rep.googleapis.com/v1/... A value like `attacker.com#` causes the HTTP client to contact `secretmanager.attacker.com` (the fragment truncates the real hostname), forwarding the GCP Bearer token to an attacker-controlled host. A value containing `?` produces a similar redirect via the query string. Fix: add a regex guard in the `Reference` constructor (src/reference.ts) that rejects any location value that does not match the GCP region format `^[a-z]+-[a-z0-9]+$` (e.g. "us-central1", "europe-west4"). The check runs after the switch block so it covers every code path that can set `this.location`. Four new test cases are added to tests/reference.test.ts covering `#`, `?`, `@`, and upper-case injection payloads.
|
Flagging the security context for prioritization: A consumer workflow that passes any user-influenced value into the The attack requires no interaction beyond the workflow run itself:
This has been reported to Google Security (OSS VRP). The fix in this PR (regex validation of |
|
Hi, just checking in — is there anything needed from our side to move this forward? Happy to rebase or address any review feedback. |
|
Gentle follow-up @verbanicm. This is one of a small cluster of same-class fixes across the org's actions where an unvalidated user-influenced input is substituted into the API endpoint, routing the request (with the GCP access token) to an attacker-controlled host — here via |
Summary
Vulnerability
The
locationfield from thesecrets:action input is used verbatim in URL construction insidesrc/client.ts:No format validation is applied to
ref.locationanywhere in the parsing path (src/reference.ts). This means an attacker who can influence thesecrets:input (e.g. through a compromised dependency that writes workflow files, or a pull-request workflow with insufficient trust controls) can inject characters that manipulate the constructed URL:attacker.com#https://secretmanager.attacker.com#.rep.googleapis.com/v1/...attacker.comattacker.com?x=https://secretmanager.attacker.com?x=.rep.googleapis.com/v1/...attacker.comattacker.com@evil.hostevil.hostIn all cases the HTTP client sends a GCP Bearer token (
Authorization: Bearer <token>) to the attacker-controlled host.Root Cause
src/reference.ts— theReferenceconstructor — splits and trims the secret reference string but does not validate the extractedlocationsegment against any expected format before storing it.Fix
Added a regex guard at the end of the
Referenceconstructor, covering every code path that can setthis.location:This matches the real GCP region naming convention (
us-central1,europe-west4,northamerica-northeast1, etc.) and rejects any value containing#,?,@, uppercase letters, or other URL-special characters before it can reach the HTTP client.Four new test cases were added to
tests/reference.test.tscovering#,?,@, and uppercase injection payloads. All 24 tests pass.Test plan
npm test— 24/24 pass, including 4 new injection test casesus-central1,europe-west4) still parse correctly (covered by existing testout:projects/my-project/locations/my-location/secrets/my-secret)attacker.com#/my-secret/latest→ throwsInvalid location formatattacker.com?foo=/my-secret/latest→ throwsInvalid location formatattacker.com@evil.host/my-secret/latest→ throwsInvalid location formatUS-CENTRAL1/my-secret/latest→ throwsInvalid location formatFiles changed
src/reference.ts— validation guard added toReferenceconstructortests/reference.test.ts— 4 new test cases for malicious location payloads