From 5fb80d69e3866dfccb0aebd0a7818788b9bb1df6 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Mon, 3 Aug 2026 11:01:53 +0530 Subject: [PATCH 1/6] SECOPS-25213 [Salesforce Marketing Cloud]: reject subdomain host/path 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) --- .../_tests_/subdomainValidation.test.ts | 100 ++++++++++++++++++ .../apiEvent/index.ts | 2 + .../contact/index.ts | 2 + .../salesforce-marketing-cloud/index.ts | 2 + .../sfmc-operations.ts | 24 +++++ 5 files changed, 130 insertions(+) create mode 100644 packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts new file mode 100644 index 00000000000..75626dfa6db --- /dev/null +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts @@ -0,0 +1,100 @@ +import nock from 'nock' +import { createTestEvent, createTestIntegration, PayloadValidationError } from '@segment/actions-core' +import Definition from '../index' +import { Settings } from '../generated-types' +import { validateSubdomain } from '../sfmc-operations' + +const testDestination = createTestIntegration(Definition) +const timestamp = '2022-05-12T15:21:15.449Z' + +const baseSettings: Settings = { + subdomain: 'test123', + client_id: 'test123', + client_secret: 'test123', + account_id: 'test123' +} + +// Subdomain values that an attacker could use to redirect requests to a host they +// control, thereby exfiltrating the OAuth client secret / bearer token. See SECOPS-25213. +const MALICIOUS_SUBDOMAINS = [ + 'mc123.attacker.com/', // path injection - host stays SFMC but path forwards elsewhere + 'attacker.com/', // resolves to https://attacker.com/.rest.marketingcloudapis.com/... + 'mc123.attacker.com', // dot lets the attacker prepend their own host + 'mc123@attacker.com', // userinfo trick - real host becomes attacker.com + 'mc123.attacker.com#', // fragment injection + 'mc123.attacker.com?', // query injection + 'mc123:8080', // port injection + 'mc 123', // whitespace + 'mc123/../../evil', // path traversal + '' +] + +describe('Salesforce Marketing Cloud - subdomain validation', () => { + afterEach(() => { + nock.cleanAll() + }) + + describe('validateSubdomain()', () => { + it('accepts a valid tenant subdomain', () => { + expect(validateSubdomain('mc563885gzs27c5t9-63k636ttgm')).toBe('mc563885gzs27c5t9-63k636ttgm') + expect(validateSubdomain('test123')).toBe('test123') + }) + + it.each(MALICIOUS_SUBDOMAINS)('rejects malicious subdomain %p', (subdomain) => { + expect(() => validateSubdomain(subdomain)).toThrow(PayloadValidationError) + }) + + it('rejects non-string values', () => { + expect(() => validateSubdomain(undefined)).toThrow(PayloadValidationError) + expect(() => validateSubdomain(null)).toThrow(PayloadValidationError) + }) + }) + + describe('contact action', () => { + it('does not forward the request to an attacker-controlled host', async () => { + const settings: Settings = { ...baseSettings, subdomain: 'mc123.attacker.com/' } + + // If the guard is removed, the request would go here instead of throwing. + const attackerScope = nock('https://mc123.attacker.com').post(/.*/).reply(200, {}) + + const event = createTestEvent({ + timestamp, + type: 'identify', + traits: { contactKey: 'ericForman15' } + }) + + await expect( + testDestination.testAction('contact', { + event, + settings, + mapping: { contactKey: { '@path': '$.traits.contactKey' } } + }) + ).rejects.toThrow(PayloadValidationError) + + expect(attackerScope.isDone()).toBe(false) + }) + }) + + describe('apiEvent action', () => { + it('rejects a malicious subdomain before making a request', async () => { + const settings: Settings = { ...baseSettings, subdomain: 'attacker.com/' } + const attackerScope = nock('https://attacker.com').post(/.*/).reply(200, {}) + + const event = createTestEvent({ timestamp, type: 'track' }) + + await expect( + testDestination.testAction('apiEvent', { + event, + settings, + mapping: { + eventDefinitionKey: 'event-definition-key', + contactKey: 'contact-key', + data: { key: 'value' } + } + }) + ).rejects.toThrow(PayloadValidationError) + + expect(attackerScope.isDone()).toBe(false) + }) + }) +}) diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/apiEvent/index.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/apiEvent/index.ts index 01a2cb71ef6..7be01df8f83 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/apiEvent/index.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/apiEvent/index.ts @@ -3,6 +3,7 @@ import type { Settings } from '../generated-types' import { eventDefinitionKey, contactKeyAPIEvent, eventData } from '../sfmc-properties' import type { Payload } from './generated-types' import { SALESFORCE_MARKETING_CLOUD_DATA_API_VERSION } from '../versioning-info' +import { validateSubdomain } from '../sfmc-operations' const action: ActionDefinition = { title: 'Send API Event', @@ -13,6 +14,7 @@ const action: ActionDefinition = { data: eventData }, perform: (request, { settings, payload }) => { + validateSubdomain(settings.subdomain) return request( `https://${settings.subdomain}.rest.marketingcloudapis.com/interaction/${SALESFORCE_MARKETING_CLOUD_DATA_API_VERSION}/events`, { diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts index 0b3cc87a0d7..317940aa2c3 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts @@ -3,6 +3,7 @@ import type { Settings } from '../generated-types' import { contactKey } from '../sfmc-properties' import type { Payload } from './generated-types' import { SALESFORCE_MARKETING_CLOUD_DATA_API_VERSION } from '../versioning-info' +import { validateSubdomain } from '../sfmc-operations' const action: ActionDefinition = { title: 'Create Contact', @@ -12,6 +13,7 @@ const action: ActionDefinition = { contactKey: { ...contactKey, required: true } }, perform: (request, { settings, payload }) => { + validateSubdomain(settings.subdomain) return request( `https://${settings.subdomain}.rest.marketingcloudapis.com/contacts/${SALESFORCE_MARKETING_CLOUD_DATA_API_VERSION}/contacts`, { diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts index 57d762c20a0..a29917e60dd 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts @@ -11,6 +11,7 @@ import dataExtensionV2 from './dataExtensionV2' import asyncDataExtension from './asyncDataExtension' import contactDataExtensionV2 from './contactDataExtensionV2' import { SALESFORCE_MARKETING_CLOUD_AUTH_API_VERSION } from './versioning-info' +import { validateSubdomain } from './sfmc-operations' interface RefreshTokenResponse { access_token: string @@ -54,6 +55,7 @@ const destination: DestinationDefinition = { } }, refreshAccessToken: async (request, { settings }) => { + validateSubdomain(settings.subdomain) const baseUrl = `https://${settings.subdomain}.auth.marketingcloudapis.com/${SALESFORCE_MARKETING_CLOUD_AUTH_API_VERSION}/token` const res = await request(`${baseUrl}`, { method: 'POST', diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts index 4efdfb34316..b8c7072cd6e 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts @@ -4,6 +4,7 @@ import { JSONLikeObject, ModifiedResponse, IntegrationError, + PayloadValidationError, ActionHookResponse, DynamicFieldResponse, DynamicFieldError, @@ -22,6 +23,24 @@ import { SALESFORCE_MARKETING_CLOUD_DATA_API_VERSION } from './versioning-info' +// 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 it must be restricted to characters that +// are valid in a DNS label - letters, digits and hyphens. 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 +} + function generateRows(payloads: payload_dataExtension[] | payload_contactDataExtension[]): Record[] { const rows: Record[] = [] payloads.forEach((payload: payload_dataExtension | payload_contactDataExtension) => { @@ -70,6 +89,7 @@ export async function asyncUpsertRowsV2( 400 ) } + validateSubdomain(subdomain) // Use flattened rows for async API const rows = generateFlattenedRows(payloads) const response = await request( @@ -96,6 +116,7 @@ export function upsertRows( 400 ) } + validateSubdomain(subdomain) const rows = generateRows(payloads) if (key) { return request( @@ -130,6 +151,7 @@ export function upsertRowsV2( ) } + validateSubdomain(subdomain) const rows = generateRows(payloads) return request( `https://${subdomain}.rest.marketingcloudapis.com/hub/${SALESFORCE_MARKETING_CLOUD_HUB_API_VERSION}/dataevents/${dataExtensionId}/rowset`, @@ -147,6 +169,7 @@ export async function executeUpsertWithMultiStatus( dataExtensionId?: string, statsContext?: StatsContext ): Promise { + validateSubdomain(subdomain) const multiStatusResponse = new MultiStatusResponse() let response: ModifiedResponse | undefined const rows = generateRows(payloads) @@ -294,6 +317,7 @@ const getAccessToken = async ( request: RequestClient, settings: Settings ): Promise<{ accessToken: string; soapInstanceUrl: string }> => { + validateSubdomain(settings.subdomain) const baseUrl = `https://${settings.subdomain}.auth.marketingcloudapis.com/${SALESFORCE_MARKETING_CLOUD_AUTH_API_VERSION}/token` const res = await request(`${baseUrl}`, { method: 'POST', From cbdc690e7dc7ccd2de3ea82d249a6bc9c8172b03 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Mon, 3 Aug 2026 16:20:11 +0530 Subject: [PATCH 2/6] [Salesforce Marketing Cloud] Fix inaccurate threat-model comment in subdomain 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) --- .../_tests_/subdomainValidation.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts index 75626dfa6db..a7cbf079bf1 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts @@ -17,7 +17,7 @@ const baseSettings: Settings = { // Subdomain values that an attacker could use to redirect requests to a host they // control, thereby exfiltrating the OAuth client secret / bearer token. See SECOPS-25213. const MALICIOUS_SUBDOMAINS = [ - 'mc123.attacker.com/', // path injection - host stays SFMC but path forwards elsewhere + 'mc123.attacker.com/', // trailing slash makes the effective host mc123.attacker.com (attacker-controlled); the SFMC suffix becomes part of the path 'attacker.com/', // resolves to https://attacker.com/.rest.marketingcloudapis.com/... 'mc123.attacker.com', // dot lets the attacker prepend their own host 'mc123@attacker.com', // userinfo trick - real host becomes attacker.com From 771ad1a109b5450f1df5062177a2eee93d846066 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Mon, 3 Aug 2026 16:21:02 +0530 Subject: [PATCH 3/6] [Salesforce Marketing Cloud] Clarify subdomain-validation comment matches 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) --- .../salesforce-marketing-cloud/sfmc-operations.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts index b8c7072cd6e..051140d10a4 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts @@ -25,11 +25,13 @@ import { // 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 it must be restricted to characters that -// are valid in a DNS label - letters, digits and hyphens. 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. +// 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 { From 3e571bced3c3295599192129c677047476e63bef Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Mon, 3 Aug 2026 16:59:49 +0530 Subject: [PATCH 4/6] [Salesforce Marketing Cloud] Add refresh-token subdomain-injection test 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) --- .../_tests_/subdomainValidation.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts index a7cbf079bf1..db1e0ded0d8 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts @@ -50,6 +50,30 @@ describe('Salesforce Marketing Cloud - subdomain validation', () => { }) }) + describe('refreshAccessToken', () => { + it('does not forward the client secret to an attacker-controlled auth host', async () => { + const settings: Settings = { ...baseSettings, subdomain: 'sfmc-credential-capture.example/' } + + // The token refresh POSTs client_id/client_secret. If the guard is removed, the + // request would land here (host becomes sfmc-credential-capture.example) instead + // of throwing. This is the path exercised by the SECOPS-25213 PoC. + const attackerScope = nock('https://sfmc-credential-capture.example').post(/.*/).reply(200, { + access_token: 'attacker-controlled-token' + }) + + await expect( + testDestination.refreshAccessToken(settings, { + refreshToken: 'refresh', + accessToken: 'access', + clientId: settings.client_id, + clientSecret: settings.client_secret + }) + ).rejects.toThrow(PayloadValidationError) + + expect(attackerScope.isDone()).toBe(false) + }) + }) + describe('contact action', () => { it('does not forward the request to an attacker-controlled host', async () => { const settings: Settings = { ...baseSettings, subdomain: 'mc123.attacker.com/' } From a011b7b71f7c885651b51fa694f02c4ddcfa85ce Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Mon, 10 Aug 2026 11:15:38 +0530 Subject: [PATCH 5/6] [Salesforce Marketing Cloud] Validate subdomain at settings-save via testAuthentication Adds a testAuthentication handler that runs validateSubdomain() when a customer saves their settings, so an invalid or injection subdomain is rejected immediately with a clear message instead of surfacing later as a failed event delivery. Addresses PR review feedback on SECOPS-25213. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../_tests_/subdomainValidation.test.ts | 16 ++++++++++++++++ .../salesforce-marketing-cloud/index.ts | 5 +++++ 2 files changed, 21 insertions(+) diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts index db1e0ded0d8..9dfb5adcaa4 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts @@ -50,6 +50,22 @@ describe('Salesforce Marketing Cloud - subdomain validation', () => { }) }) + describe('testAuthentication', () => { + it('accepts a valid subdomain', async () => { + await expect(testDestination.testAuthentication(baseSettings)).resolves.not.toThrow() + }) + + // testAuthentication runs when the customer saves their settings. The core wrapper + // re-throws as a generic Error but preserves the message, so the customer sees the + // subdomain problem immediately instead of via a later failed event delivery. + it.each(MALICIOUS_SUBDOMAINS)('rejects malicious subdomain %p at settings-save time', async (subdomain) => { + const settings: Settings = { ...baseSettings, subdomain } + await expect(testDestination.testAuthentication(settings)).rejects.toThrow( + 'Invalid Salesforce Marketing Cloud subdomain' + ) + }) + }) + describe('refreshAccessToken', () => { it('does not forward the client secret to an attacker-controlled auth host', async () => { const settings: Settings = { ...baseSettings, subdomain: 'sfmc-credential-capture.example/' } diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts index a29917e60dd..a31b13ed51c 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts @@ -54,6 +54,11 @@ const destination: DestinationDefinition = { required: true } }, + testAuthentication: async (_request, { settings }) => { + // Validate the subdomain at settings-save time so the customer gets a clear, + // immediate error instead of a failed event delivery later. See SECOPS-25213. + validateSubdomain(settings.subdomain) + }, refreshAccessToken: async (request, { settings }) => { validateSubdomain(settings.subdomain) const baseUrl = `https://${settings.subdomain}.auth.marketingcloudapis.com/${SALESFORCE_MARKETING_CLOUD_AUTH_API_VERSION}/token` From 3ca906a6ef43d2c728b4c04e00aa62e3f331cdeb Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Mon, 10 Aug 2026 16:04:51 +0530 Subject: [PATCH 6/6] [Salesforce Marketing Cloud] Validate subdomain only at settings-save Per PR review: keep subdomain validation solely in testAuthentication and drop it from every per-request path (refreshAccessToken, getAccessToken, contact/apiEvent perform, and the data-extension write helpers). Validating at settings-save means an invalid or injection subdomain is rejected before it can be stored, while removing the per-event check guarantees no pre-existing configuration can start failing delivery because of the regex. Tests updated to cover the validator and testAuthentication only. See SECOPS-25213. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../_tests_/subdomainValidation.test.ts | 89 ++----------------- .../apiEvent/index.ts | 2 - .../contact/index.ts | 2 - .../salesforce-marketing-cloud/index.ts | 1 - .../sfmc-operations.ts | 21 +++-- 5 files changed, 17 insertions(+), 98 deletions(-) diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts index 9dfb5adcaa4..4e10756a1eb 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts @@ -1,11 +1,9 @@ -import nock from 'nock' -import { createTestEvent, createTestIntegration, PayloadValidationError } from '@segment/actions-core' +import { createTestIntegration, PayloadValidationError } from '@segment/actions-core' import Definition from '../index' import { Settings } from '../generated-types' import { validateSubdomain } from '../sfmc-operations' const testDestination = createTestIntegration(Definition) -const timestamp = '2022-05-12T15:21:15.449Z' const baseSettings: Settings = { subdomain: 'test123', @@ -30,10 +28,6 @@ const MALICIOUS_SUBDOMAINS = [ ] describe('Salesforce Marketing Cloud - subdomain validation', () => { - afterEach(() => { - nock.cleanAll() - }) - describe('validateSubdomain()', () => { it('accepts a valid tenant subdomain', () => { expect(validateSubdomain('mc563885gzs27c5t9-63k636ttgm')).toBe('mc563885gzs27c5t9-63k636ttgm') @@ -50,14 +44,17 @@ describe('Salesforce Marketing Cloud - subdomain validation', () => { }) }) + // The subdomain is validated only at settings-save time (testAuthentication), so an + // invalid or injection value is rejected before it can ever be stored - the customer + // gets a clear, immediate error. We intentionally do NOT re-validate on every event + // to avoid breaking delivery for any pre-existing config. See SECOPS-25213. describe('testAuthentication', () => { it('accepts a valid subdomain', async () => { await expect(testDestination.testAuthentication(baseSettings)).resolves.not.toThrow() }) - // testAuthentication runs when the customer saves their settings. The core wrapper - // re-throws as a generic Error but preserves the message, so the customer sees the - // subdomain problem immediately instead of via a later failed event delivery. + // The core wrapper re-throws as a generic Error but preserves the message, so the + // customer sees the subdomain problem immediately at save time. it.each(MALICIOUS_SUBDOMAINS)('rejects malicious subdomain %p at settings-save time', async (subdomain) => { const settings: Settings = { ...baseSettings, subdomain } await expect(testDestination.testAuthentication(settings)).rejects.toThrow( @@ -65,76 +62,4 @@ describe('Salesforce Marketing Cloud - subdomain validation', () => { ) }) }) - - describe('refreshAccessToken', () => { - it('does not forward the client secret to an attacker-controlled auth host', async () => { - const settings: Settings = { ...baseSettings, subdomain: 'sfmc-credential-capture.example/' } - - // The token refresh POSTs client_id/client_secret. If the guard is removed, the - // request would land here (host becomes sfmc-credential-capture.example) instead - // of throwing. This is the path exercised by the SECOPS-25213 PoC. - const attackerScope = nock('https://sfmc-credential-capture.example').post(/.*/).reply(200, { - access_token: 'attacker-controlled-token' - }) - - await expect( - testDestination.refreshAccessToken(settings, { - refreshToken: 'refresh', - accessToken: 'access', - clientId: settings.client_id, - clientSecret: settings.client_secret - }) - ).rejects.toThrow(PayloadValidationError) - - expect(attackerScope.isDone()).toBe(false) - }) - }) - - describe('contact action', () => { - it('does not forward the request to an attacker-controlled host', async () => { - const settings: Settings = { ...baseSettings, subdomain: 'mc123.attacker.com/' } - - // If the guard is removed, the request would go here instead of throwing. - const attackerScope = nock('https://mc123.attacker.com').post(/.*/).reply(200, {}) - - const event = createTestEvent({ - timestamp, - type: 'identify', - traits: { contactKey: 'ericForman15' } - }) - - await expect( - testDestination.testAction('contact', { - event, - settings, - mapping: { contactKey: { '@path': '$.traits.contactKey' } } - }) - ).rejects.toThrow(PayloadValidationError) - - expect(attackerScope.isDone()).toBe(false) - }) - }) - - describe('apiEvent action', () => { - it('rejects a malicious subdomain before making a request', async () => { - const settings: Settings = { ...baseSettings, subdomain: 'attacker.com/' } - const attackerScope = nock('https://attacker.com').post(/.*/).reply(200, {}) - - const event = createTestEvent({ timestamp, type: 'track' }) - - await expect( - testDestination.testAction('apiEvent', { - event, - settings, - mapping: { - eventDefinitionKey: 'event-definition-key', - contactKey: 'contact-key', - data: { key: 'value' } - } - }) - ).rejects.toThrow(PayloadValidationError) - - expect(attackerScope.isDone()).toBe(false) - }) - }) }) diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/apiEvent/index.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/apiEvent/index.ts index 7be01df8f83..01a2cb71ef6 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/apiEvent/index.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/apiEvent/index.ts @@ -3,7 +3,6 @@ import type { Settings } from '../generated-types' import { eventDefinitionKey, contactKeyAPIEvent, eventData } from '../sfmc-properties' import type { Payload } from './generated-types' import { SALESFORCE_MARKETING_CLOUD_DATA_API_VERSION } from '../versioning-info' -import { validateSubdomain } from '../sfmc-operations' const action: ActionDefinition = { title: 'Send API Event', @@ -14,7 +13,6 @@ const action: ActionDefinition = { data: eventData }, perform: (request, { settings, payload }) => { - validateSubdomain(settings.subdomain) return request( `https://${settings.subdomain}.rest.marketingcloudapis.com/interaction/${SALESFORCE_MARKETING_CLOUD_DATA_API_VERSION}/events`, { diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts index 317940aa2c3..0b3cc87a0d7 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/contact/index.ts @@ -3,7 +3,6 @@ import type { Settings } from '../generated-types' import { contactKey } from '../sfmc-properties' import type { Payload } from './generated-types' import { SALESFORCE_MARKETING_CLOUD_DATA_API_VERSION } from '../versioning-info' -import { validateSubdomain } from '../sfmc-operations' const action: ActionDefinition = { title: 'Create Contact', @@ -13,7 +12,6 @@ const action: ActionDefinition = { contactKey: { ...contactKey, required: true } }, perform: (request, { settings, payload }) => { - validateSubdomain(settings.subdomain) return request( `https://${settings.subdomain}.rest.marketingcloudapis.com/contacts/${SALESFORCE_MARKETING_CLOUD_DATA_API_VERSION}/contacts`, { diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts index a31b13ed51c..c3f9abb6376 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/index.ts @@ -60,7 +60,6 @@ const destination: DestinationDefinition = { validateSubdomain(settings.subdomain) }, refreshAccessToken: async (request, { settings }) => { - validateSubdomain(settings.subdomain) const baseUrl = `https://${settings.subdomain}.auth.marketingcloudapis.com/${SALESFORCE_MARKETING_CLOUD_AUTH_API_VERSION}/token` const res = await request(`${baseUrl}`, { method: 'POST', diff --git a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts index 50d25c9e012..cad5a586758 100644 --- a/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/sfmc-operations.ts @@ -28,12 +28,16 @@ import { // 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. +// in a DNS label - letters, digits and hyphens. 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. +// +// This is enforced only at settings-save time (testAuthentication), so an invalid +// value can never be stored - we deliberately do NOT re-validate on every event, to +// avoid breaking delivery for any pre-existing config. The pattern 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. +// See SECOPS-25213. const SUBDOMAIN_PATTERN = /^[a-zA-Z0-9-]+$/ export function validateSubdomain(subdomain: unknown): string { @@ -93,7 +97,6 @@ export async function asyncUpsertRowsV2( 400 ) } - validateSubdomain(subdomain) // Use flattened rows for async API const rows = generateFlattenedRows(payloads) const response = await request( @@ -120,7 +123,6 @@ export function upsertRows( 400 ) } - validateSubdomain(subdomain) const rows = generateRows(payloads) if (key) { return request( @@ -155,7 +157,6 @@ export function upsertRowsV2( ) } - validateSubdomain(subdomain) const rows = generateRows(payloads) return request( `https://${subdomain}.rest.marketingcloudapis.com/hub/${SALESFORCE_MARKETING_CLOUD_HUB_API_VERSION}/dataevents/${dataExtensionId}/rowset`, @@ -173,7 +174,6 @@ export async function executeUpsertWithMultiStatus( dataExtensionId?: string, statsContext?: StatsContext ): Promise { - validateSubdomain(subdomain) const multiStatusResponse = new MultiStatusResponse() let response: ModifiedResponse | undefined const rows = generateRows(payloads) @@ -321,7 +321,6 @@ const getAccessToken = async ( request: RequestClient, settings: Settings ): Promise<{ accessToken: string; soapInstanceUrl: string }> => { - validateSubdomain(settings.subdomain) const baseUrl = `https://${settings.subdomain}.auth.marketingcloudapis.com/${SALESFORCE_MARKETING_CLOUD_AUTH_API_VERSION}/token` const res = await request(`${baseUrl}`, { method: 'POST',