From 4ab9b58dfd261fe730aaf263c196fa7bcffdf648 Mon Sep 17 00:00:00 2001 From: jyotir-aditya Date: Mon, 26 Oct 2026 07:35:44 +0530 Subject: [PATCH] oauthex: parse WWW-Authenticate in linear time splitChallenges looked ahead from every comma by trimming and scanning the rest of the header, so parsing took time quadratic in the header length. A header made of 100 KB of commas took about 5.7s to parse, and the work does not observe context cancellation. Compute the lookahead once per run of commas and blanks, and read only the next token to decide whether a comma starts an auth-param (startsWithAuthParam). The result is unchanged except for a comma inside the name before the '=', such as `x, a,b=c`: a token cannot contain a comma (RFC 9110, section 5.6.2), so that comma now ends the challenge. Add cases to TestSplitChallenges and TestParseWWWAuthenticateLinearTime, which checks that a 100 KB run of commas parses within a deadline. Fixes #1455 --- oauthex/resource_meta.go | 42 +++++++++++++++++++++--------- oauthex/resource_meta_test.go | 49 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/oauthex/resource_meta.go b/oauthex/resource_meta.go index 880182bc2..2869c82f9 100644 --- a/oauthex/resource_meta.go +++ b/oauthex/resource_meta.go @@ -180,6 +180,11 @@ func splitChallenges(header string) ([]string, error) { inQuotes := false escaped := false start := 0 + // Every comma in a run of commas and blanks sees the same text after the + // run, so the lookahead is computed once per run. Rescanning per comma + // makes a long header take quadratic time. + runEnd := 0 + runIsParam := false for i, r := range header { if escaped { // The previous backslash began a quoted-pair, so this character is @@ -198,20 +203,13 @@ func splitChallenges(header string) ([]string, error) { // This is a potential challenge separator. // A new challenge does not start with `key=value`. // We check if the part after the comma looks like a parameter. - lookahead := trimEmptyListElements(header[i+1:]) - eqPos := strings.Index(lookahead, "=") - - isParam := false - if eqPos > 0 { - // Check if the part before '=' is a single token (no spaces). - // Whitespace between the token and '=' is allowed (BWS, RFC 9110 section 11.2). - token := strings.TrimRight(lookahead[:eqPos], " \t") - if strings.IndexFunc(token, unicode.IsSpace) == -1 { - isParam = true - } + if i >= runEnd { + lookahead := trimEmptyListElements(header[i+1:]) + runEnd = len(header) - len(lookahead) + runIsParam = startsWithAuthParam(lookahead) } - if !isParam { + if !runIsParam { // The part after the comma does not look like a parameter, // so this comma separates challenges. challenges = append(challenges, header[start:i]) @@ -224,6 +222,26 @@ func splitChallenges(header string) ([]string, error) { return challenges, nil } +// startsWithAuthParam reports whether s begins with a token followed by '='. +// Whitespace between the token and '=' is allowed (BWS, RFC 9110 section 11.2). +// It reads no further than that token, so its cost does not depend on the +// length of the rest of the header. +func startsWithAuthParam(s string) bool { + for i, r := range s { + switch { + case r == '=': + return i > 0 + case r == ',': + return false + case r == ' ' || r == '\t': + return strings.HasPrefix(strings.TrimLeft(s[i:], " \t"), "=") + case unicode.IsSpace(r): + return false + } + } + return false +} + // parseSingleChallenge parses a string containing exactly one challenge. // challenge = auth-scheme [ 1*SP ( token68 / #auth-param ) ] func parseSingleChallenge(s string) (Challenge, error) { diff --git a/oauthex/resource_meta_test.go b/oauthex/resource_meta_test.go index c262bffc7..c6678006c 100644 --- a/oauthex/resource_meta_test.go +++ b/oauthex/resource_meta_test.go @@ -9,7 +9,9 @@ import ( "net/http" "net/http/httptest" "reflect" + "strings" "testing" + "time" ) func TestSplitChallenges(t *testing.T) { @@ -68,6 +70,31 @@ func TestSplitChallenges(t *testing.T) { input: "", want: []string{""}, }, + { + name: "challenges without params", + input: `Basic,Bearer`, + want: []string{`Basic`, `Bearer`}, + }, + { + name: "blank between token and equals", + input: "Bearer a=1, b \t=2", + want: []string{"Bearer a=1, b \t=2"}, + }, + { + name: "whitespace inside the name starts a new challenge", + input: `Basic realm="a", Bearer x y=1`, + want: []string{`Basic realm="a"`, ` Bearer x y=1`}, + }, + { + name: "non-blank whitespace before equals starts a new challenge", + input: "Basic, Bearer\u00a0=1", + want: []string{"Basic", " Bearer\u00a0=1"}, + }, + { + name: "comma inside the name starts a new challenge", + input: `Bearer x, a,b=c`, + want: []string{`Bearer x`, ` a,b=c`}, + }, } for _, tt := range tests { @@ -83,6 +110,28 @@ func TestSplitChallenges(t *testing.T) { } } +// The lookahead after a comma must not rescan the rest of the header, or a +// long header from a server takes quadratic time that no context deadline can +// interrupt. The input is kept small: the test only needs the old code to take +// several times longer than the deadline, and it must not load the machine, +// because other packages' tests run in parallel with it. +func TestParseWWWAuthenticateLinearTime(t *testing.T) { + header := "Bearer " + strings.Repeat(",", 100_000) + done := make(chan error, 1) + go func() { + _, err := ParseWWWAuthenticate([]string{header}) + done <- err + }() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(2 * time.Second): + t.Fatalf("ParseWWWAuthenticate did not finish within 2s on a %d byte header", len(header)) + } +} + // A quoted string may end in an escaped backslash. The quote after it closes // the string, so the comma that follows still separates two challenges and // the Bearer challenge the auth handlers look for is not lost.