Skip to content

AuthChallengeParser silently accepts a stray trailing token after a valid auth-param #111

@OmarAlJarrah

Description

@OmarAlJarrah

Summary

AuthChallengeParser accepts a malformed challenge of the shape Scheme key=token <stray-token>
a stray bare token after an otherwise valid key=token auth-param, with no comma separating them. It
neither rejects the challenge nor surfaces the stray token; it silently discards the trailing token
and emits the challenge as if it were well-formed. The impact is bounded and low, but the behaviour
is not pinned by any regression test, so it is free to drift.

Evidence

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/AuthChallengeParser.kt, in
parseChallenge. After the first auth-param is parsed, the continuation loop only keeps consuming
params when the next non-whitespace char is a comma:

AuthChallengeParser.kt:98-100:

while (true) {
    cursor.skipOws()
    if (!cursor.hasMore() || cursor.peek() != ',') break

Trace for input Digest realm=value extra:

  1. parseChallenge reads scheme digest, then skipOws() (AuthChallengeParser.kt:78-79).
  2. parseAuthParamOrToken68 reads realm, consumes =, reads value value, and returns
    realm -> value (AuthChallengeParser.kt:143-177). params = {realm: value}.
  3. Back in the continuation loop, skipOws() consumes the space before extra, then
    cursor.peek() is e (not ,), so the loop breaks (AuthChallengeParser.kt:99-100).
  4. parseChallenge returns AuthenticateChallenge("digest", {realm=value})
    (AuthChallengeParser.kt:130).

The cursor is left sitting at extra. On the next outer-loop iteration
(AuthChallengeParser.kt:51-56), skipOwsAndCommas() is a no-op (no leading comma), and extra is
then read as the scheme of a second challenge — producing a phantom AuthenticateChallenge("extra", {}). So Digest realm=value extra parses into two challenges (digest with realm=value, plus a
bare extra) rather than being rejected or treated as a single malformed challenge. Either way, the
stray trailing token is accepted without complaint instead of being flagged.

Note this is distinct from the malformed-continuation case the parser does guard. The existing
regression test malformed continuation after a param does not emit a phantom challenge
(sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/auth/AuthChallengeParserTest.kt:398-413) covers
Digest realm="x", foo=)Bearer abc123 — a comma-separated continuation whose value starts with a
non-token char. The stray-token shape here has no comma and a valid preceding key=token, so
it takes a different path and is not covered.

Bounded / low impact

The parser's output feeds the auth challenge handlers only — AuthChallengeParser is referenced
solely from AuthenticateChallenge.kt and the auth package
(sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/). CompositeChallengeHandler dispatches
to the first handler whose canHandle(...) returns true and otherwise returns null
(sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/CompositeChallengeHandler.kt:28-37), so a
spurious/under-specified challenge that no handler can satisfy is simply ignored. Nothing shipped
consumes the parser output in a way that turns this lenient parse into an exploitable condition. It
is a correctness/hardening gap, not a security issue.

No regression test pins the behaviour

AuthChallengeParserTest.kt exercises whitespace tolerance around =/commas (line 69),
garbage-after-comma (line 261), the key, no-equals rewind (line 371), and the malformed
comma-continuation guard (line 398) — but there is no test for a stray bare token directly following
a valid key=token with no comma. The behaviour is therefore unspecified and can silently change.

Suggested fix

Harden the parser and pin the result with a regression test:

  • Decide and codify the intended behaviour for a stray trailing token after a valid auth-param
    (Scheme key=token <token>): either reject the malformed tail (recover via
    recoverToNextChallenge) or explicitly define and document that the trailing token starts a new
    challenge. Whichever is chosen, make it deliberate.
  • Add a regression test that feeds Digest realm=value extra (and a quoted variant such as
    Digest realm="value" extra) and asserts the pinned outcome, mirroring the style of the existing
    malformed continuation … guard at AuthChallengeParserTest.kt:398.

Labels

bug, sdk-core, tech-debt

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsdk-coresdk-core toolkit

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions