SECOPS-25213 [Salesforce Marketing Cloud]: reject subdomain host/path injection - #3915
SECOPS-25213 [Salesforce Marketing Cloud]: reject subdomain host/path injection#3915harsh-joshi99 wants to merge 5 commits into
Conversation
… injection The customer-supplied `subdomain` setting was interpolated directly into the host portion of every request URL. A value such as `attacker.com/` or `mc123@attacker.com` rewrites the effective host, causing the OAuth token request (which carries client_id/client_secret) and all bearer-authenticated calls to be sent to an attacker-controlled server. Add a central `validateSubdomain()` helper that enforces a single DNS label ([a-zA-Z0-9-]+) and throws a non-retryable PayloadValidationError otherwise. It is invoked at every point where `subdomain` reaches a URL: refreshAccessToken, getAccessToken, the contact and apiEvent perform methods, and the data-extension upsert/async/multistatus paths. Any value the guard rejects could never have formed a valid *.marketingcloudapis.com host, so no previously-working configuration is affected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR addresses SECOPS-25213 by preventing host/path injection via the Salesforce Marketing Cloud subdomain setting, ensuring credentials/tokens are never sent to attacker-controlled hosts.
Changes:
- Added a centralized
validateSubdomain()helper that rejects non-DNS-label input via a non-retryablePayloadValidationError. - Enforced subdomain validation at each URL construction point (token, contact/event actions, and data extension upserts).
- Added dedicated unit/integration tests to prove malicious subdomains fail before any outbound request is made.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts | Introduces validateSubdomain() and applies it to token + upsert request paths. |
| packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts | Validates settings.subdomain before refresh-token URL construction. |
| packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts | Validates settings.subdomain before contacts API request. |
| packages/destination-actions/src/destinations/salesforce-marketing-cloud/apiEvent/index.ts | Validates settings.subdomain before events API request. |
| packages/destination-actions/src/destinations/salesforce-marketing-cloud/tests/subdomainValidation.test.ts | Adds validation and “no attacker host hit” regression tests. |
…ubdomain test The comment on 'mc123.attacker.com/' claimed the host stays SFMC; with the vulnerable interpolation the effective host is actually mc123.attacker.com (attacker-controlled). Comment-only change; no logic affected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ches regex Reword the SUBDOMAIN_PATTERN comment to state it restricts to DNS-label characters and intentionally does not enforce full DNS-label structure, so the doc matches the code (per PR review). Comment-only change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts:41
- The error message is endpoint-specific (mentions only
.rest.marketingcloudapis.com), butsubdomainis also used to build.auth.marketingcloudapis.comhosts in this destination. This can mislead users who copied the full auth host or a generic*.marketingcloudapis.comURL. Suggest making the guidance suffix-agnostic (e.g., “don’t include.marketingcloudapis.com”) or explicitly mentioning both.rest...and.auth....
throw new PayloadValidationError(
'Invalid Salesforce Marketing Cloud subdomain. The subdomain may only contain letters, numbers and hyphens, and must not include the ".rest.marketingcloudapis.com" part of your subdomain URL.'
)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts:42
- The error message specifically references
".rest.marketingcloudapis.com", butvalidateSubdomain()is also used for auth hosts (*.auth.marketingcloudapis.com). To avoid confusing users who copied an auth URL (or any full hostname), update the message to be host-agnostic (e.g., “do not include any dots or the marketingcloudapis.com domain suffix”) rather than naming only the REST domain.
export function validateSubdomain(subdomain: unknown): string {
if (typeof subdomain !== 'string' || !SUBDOMAIN_PATTERN.test(subdomain)) {
throw new PayloadValidationError(
'Invalid Salesforce Marketing Cloud subdomain. The subdomain may only contain letters, numbers and hyphens, and must not include the ".rest.marketingcloudapis.com" part of your subdomain URL.'
)
}
packages/destination-actions/src/destinations/salesforce-marketing-cloud/tests/subdomainValidation.test.ts:35
- These tests validate that no request reaches an attacker-controlled host, but they do not prevent real network calls if an unexpected URL is generated (e.g., if the host differs from the
nock()scope). Consider disabling outbound connections in this suite (e.g.,nock.disableNetConnect()in abeforeAll, and restoring inafterAll) to make the “never hits attacker host” guarantee deterministic and avoid flaky/off-network behavior.
afterEach(() => {
nock.cleanAll()
})
The token refresh path carries client_id/client_secret and is the most sensitive place a malicious subdomain could exfiltrate credentials. Add a test that calls refreshAccessToken with an injection subdomain and asserts it throws PayloadValidationError and never hits the attacker auth host (per PR review). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts:43
- The error message is overly specific to the “.rest.marketingcloudapis.com” suffix, but this subdomain is also used to build
.auth.marketingcloudapis.comURLs. Consider rewording to say the value must be a single subdomain label and must not include any.marketingcloudapis.comsuffix (or any URL components like scheme/host/path), so users pasting...auth...aren’t misled.
throw new PayloadValidationError(
'Invalid Salesforce Marketing Cloud subdomain. The subdomain may only contain letters, numbers and hyphens, and must not include the ".rest.marketingcloudapis.com" part of your subdomain URL.'
)
packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts:6
- Importing
validateSubdomainfromsfmc-operationsincreases coupling between action modules and a (likely) larger operations module. Consider movingvalidateSubdomain(andSUBDOMAIN_PATTERN) into a small dedicated module (e.g.sfmc-validation.ts) so actions can depend on the lightweight validator without pulling in unrelated operational code.
import { validateSubdomain } from '../sfmc-operations'
packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts:326
- Since
validateSubdomain()returns the validated string, it’s safer/clearer to assign it and use the returned value for URL construction (e.g.,const subdomain = validateSubdomain(settings.subdomain)then interpolatesubdomain). This avoids any future refactor accidentally interpolating an unvalidated value and makes the “validated before use” relationship explicit.
validateSubdomain(settings.subdomain)
const baseUrl = `https://${settings.subdomain}.auth.marketingcloudapis.com/${SALESFORCE_MARKETING_CLOUD_AUTH_API_VERSION}/token`
const res = await request<RefreshTokenResponse>(`${baseUrl}`, {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts:46
- Placing
validateSubdomain()insfmc-operations.tsincreases coupling: action modules (contact,apiEvent, and destinationindex.ts) now import an operations module that likely also contains request logic and other heavy dependencies. To keep dependencies minimal and reduce the chance of future circular imports, consider movingvalidateSubdomain()(andSUBDOMAIN_PATTERN) into a small dedicated module (e.g.subdomain.ts/validation.ts) and importing from there.
// A Salesforce Marketing Cloud subdomain is a single DNS label (a tenant-specific
// string such as "mc563885gzs27c5t9-63k636ttgm"). It is interpolated directly into
// the host portion of every request URL, so we restrict it to the characters valid
// in a DNS label - letters, digits and hyphens. This intentionally does not enforce
// full DNS-label structure (length, no leading/trailing hyphen); the goal is to block
// host/path injection, not to reject unusual-but-working subdomains. Allowing other
// characters (e.g. "/", "@", ":", ".") would let a malicious subdomain rewrite the
// request host and exfiltrate the OAuth client secret / access token to an
// attacker-controlled server. See SECOPS-25213.
const SUBDOMAIN_PATTERN = /^[a-zA-Z0-9-]+$/
export function validateSubdomain(subdomain: unknown): string {
if (typeof subdomain !== 'string' || !SUBDOMAIN_PATTERN.test(subdomain)) {
throw new PayloadValidationError(
'Invalid Salesforce Marketing Cloud subdomain. The subdomain may only contain letters, numbers and hyphens, and must not include the ".rest.marketingcloudapis.com" part of your subdomain URL.'
)
}
return subdomain
}
packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts:43
- The error message specifically references the
.rest.marketingcloudapis.comsuffix, but the validation is also applied to.auth.marketingcloudapis.comendpoints. Consider rewording to be suffix-agnostic and more directly instructive (e.g., “Provide only the tenant subdomain (single label), not a full domain or URL”). This helps reduce confusion when the failure happens on auth/token paths.
throw new PayloadValidationError(
'Invalid Salesforce Marketing Cloud subdomain. The subdomain may only contain letters, numbers and hyphens, and must not include the ".rest.marketingcloudapis.com" part of your subdomain URL.'
)
packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts:60
- To make it harder for future refactors to accidentally use an unvalidated value, consider using the return value of
validateSubdomain()(e.g., assign to a localsubdomainconstant) and interpolate that into the URL instead ofsettings.subdomain.
validateSubdomain(settings.subdomain)
const baseUrl = `https://${settings.subdomain}.auth.marketingcloudapis.com/${SALESFORCE_MARKETING_CLOUD_AUTH_API_VERSION}/token`
const res = await request<RefreshTokenResponse>(`${baseUrl}`, {
| data: eventData | ||
| }, | ||
| perform: (request, { settings, payload }) => { | ||
| validateSubdomain(settings.subdomain) |
There was a problem hiding this comment.
Can we validate it when we are saving the settings in testAuthentication so that customer receives the error early?
| // characters (e.g. "/", "@", ":", ".") would let a malicious subdomain rewrite the | ||
| // request host and exfiltrate the OAuth client secret / access token to an | ||
| // attacker-controlled server. See SECOPS-25213. | ||
| const SUBDOMAIN_PATTERN = /^[a-zA-Z0-9-]+$/ |
There was a problem hiding this comment.
can we do analysis with the settings stored in the db to ensure that this regex agrees. Otherwise, customer will complain failure while updating existing settings.
Summary
Fixes SECOPS-25213 (HackerOne) — the Salesforce Marketing Cloud (Actions) destination accepted a
subdomainsetting that could inject a host/path and forward SFMC credentials to an attacker-controlled server.The customer-supplied
subdomainsetting was interpolated directly into the host portion of every request URL, e.g.:`https://${settings.subdomain}.auth.marketingcloudapis.com/v2/token`A value such as
attacker.example/ormc123@attacker.examplerewrites the effective host. Because the token-refresh request carriesclient_id/client_secretand every other call carries the bearer token, those secrets could be sent to a host the customer controls but Segment does not.Fix
validateSubdomain()helper that enforces a single DNS label (/^[a-zA-Z0-9-]+$/) and throws a non-retryablePayloadValidationErrorfor anything else (/,@,:,., whitespace, empty, etc.).subdomainreaches a URL:refreshAccessToken(index.ts) andgetAccessToken(sfmc-operations.ts) — the client-secret paths (the latter also guards all dynamic-field / hook lookups)contactandapiEventperformmethods — the bearer-token pathsasyncUpsertRowsV2,upsertRows,upsertRowsV2,executeUpsertWithMultiStatus— data-extension write pathsBackward compatibility
No previously-working configuration is affected. For a config to have ever succeeded,
<subdomain>.rest.marketingcloudapis.comhad to be a resolvable hostname, which by definition restricts the subdomain to letters, digits and hyphens. The regex is actually slightly more permissive than strict DNS, so every value it rejects either (a) never formed a valid*.marketingcloudapis.comhost, or (b) is the attack itself.Testing
Stage Testing Doc
_tests_/subdomainValidation.test.ts— 14 cases: valid subdomains accepted; 10 injection vectors + non-string values rejected; andcontact/apiEventintegration tests asserting the action throws and never hits the attacker host.Validation against the reporter's PoC
Ran the reporter's unmodified proof-of-concept (real Segment Actions runtime path,
subdomain: 'sfmc-credential-capture.example/') against this branch. On the vulnerable code it capturesclient_secretat the attacker host; on this branch the request is rejected before any HTTP call is constructed, so the exploit cannot reproduce:The failure occurs at the first credential-bearing step (
refreshAccessToken), before the token body is ever built — directly satisfying the reporter's stated control expectation that "a value containing/should be rejected before any credential-bearing request is made."🤖 Generated with Claude Code