diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a6c92980..8fc754a0 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -126,3 +126,4 @@ Error messages should never include raw values from sensitive sources like envir **Prevention:** 1. Avoid including raw values in error messages when the source is potentially sensitive (env vars, auth headers). 2. Use generic error messages for validation failures of sensitive data. +## 2026-04-21 - [Path Traversal via Parameter Injection in Path Segments] **Vulnerability:** Path traversal (e.g. `../`) was possible through parameters mapped to URI path segments in `CompositeExecutor` and `MCPServer`'s `encodePathSegment`. Although the input was URL-encoded via `encodeURIComponent`, literal dots (`.`) were not encoded by this function. When the decoded URI path included `../`, it allowed attackers to traverse out of the intended base path in the external API call. **Learning:** While `encodeURIComponent` encodes characters like `/`, it does not encode dots. HTTP clients typically normalize literal `../` sequences during request formulation, but they don't decode and then normalize `%2E%2E/` which protects against traversing beyond the intended URL structure while still being accurately parsed by the downstream server. **Prevention:** To prevent directory traversal attacks through URI parameters, append `.replace(/\./g, '%2E')` to `encodeURIComponent(val)` when substituting user input into URL path paths. diff --git a/src/mcp/mcp-server.ts b/src/mcp/mcp-server.ts index 9e35ac39..3a6ff368 100644 --- a/src/mcp/mcp-server.ts +++ b/src/mcp/mcp-server.ts @@ -1107,7 +1107,7 @@ export class MCPServer { */ private encodePathSegment(value: unknown): string { const val = String(value); - return val.includes('/') ? encodeURIComponent(val) : val; + return encodeURIComponent(val).replace(/\./g, '%2E'); } /** diff --git a/src/tooling/composite-executor-security.test.ts b/src/tooling/composite-executor-security.test.ts index 4f7f1ca3..97a3b0b6 100644 --- a/src/tooling/composite-executor-security.test.ts +++ b/src/tooling/composite-executor-security.test.ts @@ -67,7 +67,7 @@ describe('CompositeExecutor Security', () => { // Fixed behavior: path contains encoded "%2E%2E" // Note: encodeURIComponent('../admin/secrets') => ..%2Fadmin%2Fsecrets // The slashes inside the injected value are encoded, preventing directory traversal - const expectedFixedPath = '/users/..%2Fadmin%2Fsecrets/profile'; + const expectedFixedPath = '/users/%2E%2E%2Fadmin%2Fsecrets/profile'; expect(capturedPaths[0]).toBe(expectedFixedPath); }); diff --git a/src/tooling/composite-executor.ts b/src/tooling/composite-executor.ts index 34e467a3..9f4f6d01 100644 --- a/src/tooling/composite-executor.ts +++ b/src/tooling/composite-executor.ts @@ -171,14 +171,14 @@ export class CompositeExecutor { return template.replace(/\{(\w+)\}/g, (_, key) => { // Try direct match first if (args[key] !== undefined) { - return encodeURIComponent(String(args[key])); + return encodeURIComponent(String(args[key])).replace(/\./g, '%2E'); } // Try aliases from profile const possibleAliases = this.parameterAliases[key] || []; for (const alias of possibleAliases) { if (args[alias] !== undefined) { - return encodeURIComponent(String(args[alias])); + return encodeURIComponent(String(args[alias])).replace(/\./g, '%2E'); } }