headers icon indicating copy to clipboard operation
headers copied to clipboard

Bugs in `Authorization: Bearer …` header parsing

Open roy-work opened this issue 2 years ago • 0 comments

The following test exposes a few issues in headers::authorization::Bearer:

#[cfg(test)]
mod tests {
  #[test]
  fn test_invalid_auth_header() {
    // Malformed, should not parse.
    let value = http::HeaderValue::from_str("Bearer    foo, Bearer bar").unwrap();
    use headers::authorization::Credentials;
    let decoded = headers::authorization::Bearer::decode(&value);
    println!("decoded = {:?}", decoded);
    println!("if decoded succeeded, the token was: {:?}", decoded.as_ref().map(|b| b.token()));
    // The header is malformed, so parsing should fail:
    assert!(decoded.is_none());
  }
}

Basically, Bearer's implementation will strip the leading string "Bearer " from the value, and return the remainder for as a token. This isn't valid according to the grammar for the Bearer authn scheme. (The grammar of the token itself is restricted to a certain set of characters, for example.) In fact, the code only checks that the header starts with Bearer in debug mode (as it is done w/ a debug_assert!), so in a release build, any value will parse, including, e.g., Basic <a basic headers' credentials>.

Even where the header is valid, and should parse, the .token() determines the token by simply removing "Bearer ".len() characters from the front of the header value, which itself is not correct: Bearer foobar (n.b., the double space; this is permitted by the grammar for the header, but is not part of the token) parses as it should, but .token() returns " foobar".

Last, the debug-mode-only debug_assert! asserts that the value starts with Bearer — but auth schemes in HTTP are case insensitive, so this too will reject valid inputs. (Note: the Basic auth-scheme parser shares its own version of this issue, too.)

roy-work avatar Jun 22 '22 19:06 roy-work