Skip to content

Header names bypass CR/LF validation while header values are validated #114

@OmarAlJarrah

Description

@OmarAlJarrah

Summary

Headers.Builder validates header values for carriage return / line feed but never validates header names. Name handling only lower-cases and trims surrounding whitespace, so an embedded \r or \n inside a name passes through the model layer intact. Downstream the two reference transports then diverge: the OkHttp transport throws an unchecked IllegalArgumentException out of execute() (escaping its IOException contract), while the JDK transport silently drops the header. It is also a (minor) header-injection surface for attacker-controlled names.

Where

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/Headers.kt

  • Values are validated: validateValues (lines 325-335) rejects \r/\n and is called from every add/set that writes values (lines 163, 183, 221, 249).

  • Names are only sanitized, not validated (line 311):

    private fun sanitizeName(value: String): String = value.lowercase(Locale.US).trim()

    trim() removes only leading/trailing whitespace; an interior \r/\n survives. Every name-keyed operation (add, set, remove, get, contains, values) routes the raw name through sanitizeName with no CR/LF check. There is no validateName anywhere in the codebase.

So Headers.builder().add("X-Evil\r\nInjected", "v") builds successfully, and the malformed name is carried to the transport.

Transport divergence

  • OkHttp — sdk-transport-okhttp/src/main/kotlin/org/dexpace/sdk/transport/okhttp/internal/RequestAdapter.kt line 52 calls builder.addHeader(rawName, value) with no try/catch. OkHttp rejects an illegal name with an unchecked IllegalArgumentException. adapt(request) is invoked in OkHttpTransport.execute at line 88 (and executeAsync at line 109) before the request's try block, which only catches InterruptedIOException/IOException (OkHttpTransport.kt lines 86-101). The IAE therefore escapes execute() despite its @Throws(IOException) contract and bypasses the transport's IOException handling / retry pipeline.

  • JDK — sdk-transport-jdkhttp/.../internal/RequestAdapter.kt lines 122-132 wrap builder.header(rawName, value) in try/catch (IllegalArgumentException) and silently drop the offending header with a DEBUG log. So the same request silently loses the header instead of failing.

Same SDK-level request, two different observable outcomes depending on transport.

Misleading comments

Two adapter comments overstate the current guarantee and should be corrected once names are validated (or now, if names stay unvalidated):

  • OkHttp RequestAdapter.kt lines 49-51: "Header names/values are validated upstream by Headers.Builder (CR/LF and other illegal characters are rejected at construction)..." — names are not validated.
  • JDK RequestAdapter.kt lines 105-108: "Illegal header values (CR/LF and similar) are now rejected upstream by Headers.Builder, so a value with control characters never reaches this point..." — accurate for values, but the surrounding text implies names are covered too.

Severity

Lower than the value case: attacker-controlled header names are far rarer than attacker-controlled values, so the practical header-splitting exposure is smaller. It remains a real splitting/injection gap and a genuine transport-divergence bug (one transport hard-fails, the other silently drops), so it is worth closing.

Suggested fix

Add a validateName beside validateValues, invoked from every add/set before the name is stored. At minimum reject \r/\n (the splitting vector, illegal in every transport); optionally restrict names to RFC 7230 tchar for a stricter, fully spec-aligned model-layer contract. Then correct the two transport-adapter comments above so they no longer claim a name guarantee the model layer does not provide.

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