diff --git a/core/packages/gax/src/transcoding.ts b/core/packages/gax/src/transcoding.ts index 37b399f50e2e..94d832b56e2d 100644 --- a/core/packages/gax/src/transcoding.ts +++ b/core/packages/gax/src/transcoding.ts @@ -127,8 +127,9 @@ export function buildQueryStringComponents( if (Array.isArray(request[key])) { for (const value of request[key] as JSONObject[]) { resultList.push( - `${prefix}${encodeWithoutSlashes(key)}=${encodeWithoutSlashes( + `${prefix}${encodeWithoutSlashes(key, key)}=${encodeWithoutSlashes( value.toString(), + key, )}`, ); } @@ -138,8 +139,9 @@ export function buildQueryStringComponents( ); } else { resultList.push( - `${prefix}${encodeWithoutSlashes(key)}=${encodeWithoutSlashes( + `${prefix}${encodeWithoutSlashes(key, key)}=${encodeWithoutSlashes( request[key] === null ? 'null' : request[key]!.toString(), + key, )}`, ); } @@ -147,17 +149,40 @@ export function buildQueryStringComponents( return resultList; } -export function encodeWithSlashes(str: string): string { +// encodeWithSlashes implements the security rules for double-asterisk ("**") pattern matches. +// We split the path into segments and strictly reject any segment that is exactly "." or "..". +// This prevents path traversal attacks across any variable directory subpaths. +export function encodeWithSlashes( + str: string, + propertyName = 'resource ID', +): string { + const segments = str.split('/'); + if (segments.some(segment => segment === '.' || segment === '..')) { + throw new Error( + `Value for ${propertyName} must not contain segments that are exactly . or .. .`, + ); + } + // Percent-encode any character that is not in the unreserved set, preserving slashes. return str .split('') - .map(c => (c.match(/[-_.~0-9a-zA-Z]/) ? c : encodeURIComponent(c))) + .map(c => (c.match(/[-_.~/0-9a-zA-Z]/) ? c : encodeURIComponent(c))) .join(''); } -export function encodeWithoutSlashes(str: string): string { +// encodeWithoutSlashes implements the security rules for single-asterisk ("*") pattern matches. +// We throw a validation error if the matched value is exactly "." or "..", +// preventing resource ID level traversal attacks. +export function encodeWithoutSlashes( + str: string, + propertyName = 'resource ID', +): string { + if (str === '.' || str === '..') { + throw new Error(`Invalid value ${str} for ${propertyName}`); + } + // Percent-encode any character that is not in the unreserved set, encoding slashes too. return str .split('') - .map(c => (c.match(/[-_.~0-9a-zA-Z/]/) ? c : encodeURIComponent(c))) + .map(c => (c.match(/[-_.~0-9a-zA-Z]/) ? c : encodeURIComponent(c))) .join(''); } @@ -165,12 +190,16 @@ function escapeRegExp(str: string) { return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); } +// applyPattern extracts the wildcards from a path template pattern, uses regex to capture +// the actual parts corresponding to each wildcard (* or **), and validates them using the +// appropriate encodeWithoutSlashes / encodeWithSlashes security logic to avoid path traversal. export function applyPattern( pattern: string, fieldValue: string, + propertyName?: string, ): string | undefined { if (!pattern || pattern === '*') { - return encodeWithSlashes(fieldValue); + return encodeWithoutSlashes(fieldValue, propertyName); } if (!pattern.includes('*') && pattern !== fieldValue) { @@ -186,11 +215,42 @@ export function applyPattern( '$', ); - if (!fieldValue.match(regex)) { + const match = fieldValue.match(regex); + if (!match) { return undefined; } - return encodeWithoutSlashes(fieldValue); + // Extract and validate wildcards in the pattern (* or **) + const wildcardRegex = /\*\*|\*/g; + let wcMatch; + const wildcards: string[] = []; + while ((wcMatch = wildcardRegex.exec(pattern)) !== null) { + wildcards.push(wcMatch[0]); + } + + // Map each captured group to its respective wildcard type and validate + const capturedGroups = match.slice(1); + for (let i = 0; i < capturedGroups.length; i++) { + const groupVal = capturedGroups[i]; + const wcType = wildcards[i]; + const propName = propertyName || 'resource ID'; + if (wcType === '*') { + // Single-asterisk wildcard validation: cannot be dot or two-dots + if (groupVal === '.' || groupVal === '..') { + throw new Error(`Invalid value ${fieldValue} for ${propName}`); + } + } else if (wcType === '**') { + // Double-asterisk wildcard validation: no segments can be dot or two-dots + const segments = groupVal.split('/'); + if (segments.some(seg => seg === '.' || seg === '..')) { + throw new Error( + `Value for ${propName} must not contain segments that are exactly . or .. .`, + ); + } + } + } + + return encodeWithSlashes(fieldValue, propertyName); } function fieldToCamelCase(field: string): string { @@ -224,6 +284,7 @@ export function match( const appliedPattern = applyPattern( pattern, fieldValue === null ? 'null' : fieldValue!.toString(), + camelCasedField, ); if (appliedPattern === undefined) { return undefined; diff --git a/core/packages/gax/test/unit/transcoding.ts b/core/packages/gax/test/unit/transcoding.ts index f6c6669c6bea..d23757c6009d 100644 --- a/core/packages/gax/test/unit/transcoding.ts +++ b/core/packages/gax/test/unit/transcoding.ts @@ -366,7 +366,7 @@ describe('gRPC to HTTP transcoding', () => { encodeWithSlashes( '_.~0-9abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ/ ', ), - '_.~0-9abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ%2F%20', + '_.~0-9abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ/%20', ); }); @@ -380,10 +380,75 @@ describe('gRPC to HTTP transcoding', () => { encodeWithoutSlashes( '_.~0-9abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ/ ', ), - '_.~0-9abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ/%20', + '_.~0-9abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ%2F%20', ); }); + // Tests added as part of the security guidelines to prevent path traversal and parameter injection + describe('REST fallback security guidelines tests', () => { + // 1. Verify single-asterisk ("*") validation correctly rejects exact dot and two-dot values + it('throws error for invalid single asterisk values ("." and "..")', () => { + assert.throws(() => { + encodeWithoutSlashes('.', 'mySingleParam'); + }, /Invalid value \. for mySingleParam/); + + assert.throws(() => { + encodeWithoutSlashes('..', 'mySingleParam'); + }, /Invalid value \.\. for mySingleParam/); + + // default propertyName + assert.throws(() => { + encodeWithoutSlashes('.'); + }, /Invalid value \. for resource ID/); + }); + + // 2. Verify double-asterisk ("**") validation strictly rejects segments that are exactly dot or two-dots + it('throws error for unsafe double asterisk path traversals', () => { + assert.throws(() => { + encodeWithSlashes('a/b/../..', 'myDoubleParam'); + }, /Value for myDoubleParam must not contain segments that are exactly \. or \.\. \./); + + assert.throws(() => { + encodeWithSlashes('a/..', 'myDoubleParam'); + }, /Value for myDoubleParam must not contain segments that are exactly \. or \.\. \./); + + assert.throws(() => { + encodeWithSlashes('..', 'myDoubleParam'); + }, /Value for myDoubleParam must not contain segments that are exactly \. or \.\. \./); + + assert.throws(() => { + encodeWithSlashes('.', 'myDoubleParam'); + }, /Value for myDoubleParam must not contain segments that are exactly \. or \.\. \./); + }); + + // 3. Verify parameter injection characters are safely percent-encoded, neutralizing attacks like $httpMethod=DELETE + it('correctly percent-encodes unsafe URL characters', () => { + assert.strictEqual(encodeWithoutSlashes('foo$bar?baz#qux'), 'foo%24bar%3Fbaz%23qux'); + assert.strictEqual(encodeWithSlashes('foo$bar?baz#qux'), 'foo%24bar%3Fbaz%23qux'); + }); + + // 4. Verify transcoding matches correctly apply security restrictions on patterns with wildcards + it('validates single asterisk and double asterisk path templates within match/transcode', () => { + // test with single asterisk templates matching dot/two-dots in applyPattern + assert.throws(() => { + applyPattern('projects/*', 'projects/.', 'project'); + }, /Invalid value projects\/\. for project/); + + assert.throws(() => { + applyPattern('projects/*', 'projects/..', 'project'); + }, /Invalid value projects\/\.\. for project/); + + // test with double asterisk templates matching segments with dots in applyPattern + assert.throws(() => { + applyPattern('projects/*/locations/**', 'projects/p1/locations/..', 'location'); + }, /Value for location must not contain segments that are exactly \. or \.\. \./); + + assert.throws(() => { + applyPattern('projects/*/locations/**', 'projects/p1/locations/us/..', 'location'); + }, /Value for location must not contain segments that are exactly \. or \.\. \./); + }); + }); + it('applyPattern', () => { assert.strictEqual(applyPattern('*', 'test'), 'test'); assert.strictEqual(applyPattern('test', 'test'), 'test'); diff --git a/packages/google-cloud-dialogflow-cx/package.json b/packages/google-cloud-dialogflow-cx/package.json index 9026dad7f470..b81dfaefb41e 100644 --- a/packages/google-cloud-dialogflow-cx/package.json +++ b/packages/google-cloud-dialogflow-cx/package.json @@ -81,7 +81,8 @@ }, "pnpm": { "overrides": { - "@sinonjs/fake-timers": "15.2.1" + "@sinonjs/fake-timers": "15.2.1", + "google-gax": "link:../../core/packages/gax" } } } diff --git a/packages/google-cloud-dialogflow-cx/test/gapic_sessions_v3.ts b/packages/google-cloud-dialogflow-cx/test/gapic_sessions_v3.ts index 4d03731fef35..193a25605067 100644 --- a/packages/google-cloud-dialogflow-cx/test/gapic_sessions_v3.ts +++ b/packages/google-cloud-dialogflow-cx/test/gapic_sessions_v3.ts @@ -442,6 +442,27 @@ describe('v3.SessionsClient', () => { }); await assert.rejects(client.detectIntent(request), expectedError); }); + + // This integration test verifies that the client safely blocks and rejects path traversal attacks + // in REST fallback mode when configured with malicious session ID inputs containing dot segments. + // This implements immediate validation to prevent unauthorized resource deletion or IDOR (e.g. b/506021899). + it('rejects path traversal in session ID for fallback/REST transport mode', async () => { + const client = new sessionsModule.v3.SessionsClient({ + credentials: { client_email: 'bogus', private_key: 'bogus' }, + projectId: 'bogus', + fallback: true, + }); + await client.initialize(); + const request = generateSampleMessage( + new protos.google.cloud.dialogflow.cx.v3.DetectIntentRequest(), + ); + // Construct a malicious session path ending with dot segments (path traversal exploit payload) + request.session = 'projects/p1/locations/l1/agents/a1/sessions/..'; + await assert.rejects(client.detectIntent(request), /Invalid value .* for session/); + + request.session = 'projects/p1/locations/l1/agents/a1/sessions/.'; + await assert.rejects(client.detectIntent(request), /Invalid value .* for session/); + }); }); describe('matchIntent', () => {