diff --git a/go/.changes/unreleased/fixed-20260910-route-template-fixed-point-decode.yaml b/go/.changes/unreleased/fixed-20260910-route-template-fixed-point-decode.yaml new file mode 100644 index 0000000000..5b80e19506 --- /dev/null +++ b/go/.changes/unreleased/fixed-20260910-route-template-fixed-point-decode.yaml @@ -0,0 +1,3 @@ +kind: fixed +body: Decode routeTemplate to a fixed point (bounded pass budget) before traversal and scheme-injection checks so double- and deeper-encoded payloads cannot bypass isValidRouteTemplate. +time: 2026-09-10T07:22:00Z diff --git a/go/extensions/bazaar/facilitator.go b/go/extensions/bazaar/facilitator.go index dff393ca3d..04bfbef800 100644 --- a/go/extensions/bazaar/facilitator.go +++ b/go/extensions/bazaar/facilitator.go @@ -421,6 +421,48 @@ func ExtractDiscoveredResourceFromPaymentPayload( // Expected format: "/users/:userId", "/weather/:country/:city", "/api/v1/items". var routeTemplateRegex = regexp.MustCompile(`^/[a-zA-Z0-9_/:.\-~%]+$`) +// Maximum number of successive PathUnescape passes applied while +// canonicalizing a routeTemplate before giving up and rejecting it. +// +// A legitimate single-encoded value reaches a fixed point (further decoding +// changes nothing) after one pass. This allows a few extra passes so +// multiply-encoded traversal/injection payloads (not just doubly-encoded +// ones) are still caught, while bounding the work done on adversarial input. +const maxRouteTemplateDecodePasses = 5 + +// fullyDecodeRouteTemplate repeatedly percent-decodes value until a fixed +// point is reached (further decoding produces no change) or +// maxRouteTemplateDecodePasses is exhausted, returning the fully-canonicalized +// string. +// +// A single decode pass only catches a payload encoded exactly once (e.g. +// %2e%2e); a double- or triple-encoded payload (%252e%252e, +// %25252e%25252e, ...) would still contain literal % sequences after one +// pass and slip past a traversal/injection substring check performed on that +// partially-decoded result. Decoding to a fixed point closes that gap for any +// encoding depth, not just the double-encoded case. +// +// Returns the fully-decoded string and true, or ("", false) if any pass fails +// to parse (malformed percent-encoding) or the pass budget is exhausted +// without reaching a fixed point. +func fullyDecodeRouteTemplate(value string) (string, bool) { + decoded := value + for i := 0; i < maxRouteTemplateDecodePasses; i++ { + next, err := url.PathUnescape(decoded) + if err != nil { + return "", false + } + if next == decoded { + return decoded, true + } + decoded = next + } + // Still changing after the pass budget: either pathologically deep + // encoding or a % that never resolves to a fixed point. Either way, + // there's no safe canonical form to validate against. Reject. + return "", false +} + // isValidRouteTemplate checks whether a routeTemplate value is structurally valid. // // Expected format: ":param" segments using colon-prefixed identifiers @@ -431,8 +473,8 @@ var routeTemplateRegex = regexp.MustCompile(`^/[a-zA-Z0-9_/:.\-~%]+$`) // payment under an arbitrary URL (catalog poisoning). This enforces minimal structural requirements: // - Must be a non-empty string starting with "/" // - Must match the safe URL path character set (alphanumeric, _, :, /, ., -, ~, %) -// - Must not contain ".." (path traversal) -// - Must not contain "://" (URL injection) +// - Must not contain ".." (path traversal), at any percent-encoding depth +// - Must not contain "://" (URL injection), at any percent-encoding depth func isValidRouteTemplate(s string) bool { if s == "" { return false @@ -440,9 +482,11 @@ func isValidRouteTemplate(s string) bool { if !routeTemplateRegex.MatchString(s) { return false } - // Decode percent-encoding before traversal checks so that %2e%2e is caught. - decoded, err := url.PathUnescape(s) - if err != nil { + // Decode to a fixed point before traversal checks, so that %2e%2e, + // %252e%252e, and deeper encodings are all caught rather than just the + // single-encoded case. + decoded, ok := fullyDecodeRouteTemplate(s) + if !ok { return false } if strings.Contains(decoded, "..") { diff --git a/go/extensions/bazaar/facilitator_test.go b/go/extensions/bazaar/facilitator_test.go index 1fe22aa1cf..5fa171885a 100644 --- a/go/extensions/bazaar/facilitator_test.go +++ b/go/extensions/bazaar/facilitator_test.go @@ -4,6 +4,7 @@ package bazaar // Uses package bazaar (not bazaar_test) to access unexported functions. import ( + "net/url" "strings" "testing" @@ -55,6 +56,44 @@ func TestIsValidRouteTemplate(t *testing.T) { assert.False(t, isValidRouteTemplate("/users/%2e%2e/admin")) assert.False(t, isValidRouteTemplate("/users/%2E%2E/admin")) }) + + t.Run("rejects double-encoded traversal sequences (regression for double-decode bypass)", func(t *testing.T) { + // %25 decodes to "%", so %252e%252e decodes-once to "%2e%2e" (still + // encoded) and only decodes-twice to "..". A single-pass decode + // wouldn't see the traversal here. + assert.False(t, isValidRouteTemplate("/users/%252e%252e/admin")) + assert.False(t, isValidRouteTemplate("/users/%252E%252E/admin")) + }) + + t.Run("rejects triple-encoded traversal sequences", func(t *testing.T) { + assert.False(t, isValidRouteTemplate("/users/%25252e%25252e/admin")) + }) + + t.Run("rejects double-encoded scheme injection", func(t *testing.T) { + // %3a decodes to ":", %2f decodes to "/" — %253a%252f%252f decodes-once + // to "%3a%2f%2f" (still encoded) and decodes-twice to "://". + assert.False(t, isValidRouteTemplate("/users/javascript%253a%252f%252fevil")) + }) + + t.Run("still accepts a legitimate single percent-encoded segment", func(t *testing.T) { + // One decode pass resolves this to plain text with no further + // encoding, reaching a fixed point immediately — must not be rejected + // just for containing a "%". + assert.True(t, isValidRouteTemplate("/search/caf%C3%A9")) + }) + + t.Run("rejects a value whose percent-encoding never resolves to a fixed point", func(t *testing.T) { + // Pathologically deep encoding (more than the decode-pass budget) has + // no safe canonical form to validate — reject rather than decode + // indefinitely. ":" is re-encoded ("%3A" → "%253A" → ...) on every + // encodeURIComponent pass, so this compounds correctly (unlike "..", + // whose dots encodeURIComponent leaves untouched). + value := ":" + for i := 0; i < 8; i++ { + value = url.QueryEscape(value) + } + assert.False(t, isValidRouteTemplate("/users/x"+value+"/admin")) + }) } func TestExtractPathParams(t *testing.T) {