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..4e10756a1eb --- /dev/null +++ b/packages/destination-actions/src/destinations/salesforce-marketing-cloud/_tests_/subdomainValidation.test.ts @@ -0,0 +1,65 @@ +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 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/', // 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 + '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', () => { + 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) + }) + }) + + // 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() + }) + + // 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( + 'Invalid Salesforce Marketing Cloud subdomain' + ) + }) + }) +}) 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..c3f9abb6376 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 @@ -53,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 }) => { const baseUrl = `https://${settings.subdomain}.auth.marketingcloudapis.com/${SALESFORCE_MARKETING_CLOUD_AUTH_API_VERSION}/token` const res = await request(`${baseUrl}`, { 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 e08b541320f..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 @@ -4,6 +4,7 @@ import { JSONLikeObject, ModifiedResponse, IntegrationError, + PayloadValidationError, ActionHookResponse, DynamicFieldResponse, DynamicFieldError, @@ -24,6 +25,30 @@ import { SFMC_SOAP_CATEGORY_BATCH_SIZE_FLAGON } 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 we restrict it to the characters 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. +// +// 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 { + 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) => {