Bug 51557 - Newline during a http header field name obscures next value
Newline during a http header field name obscures next value
Product: Tomcat 7
Classification: Unclassified
Component: Connectors
PC All
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
Depends on:
  Show dependency tree
Reported: 2011-07-26 06:12 UTC by Henri Yandell
Modified: 2011-08-01 16:41 UTC (History)
0 users

Patch to stop CR & LF in the header. (747 bytes, application/octet-stream)
2011-07-26 06:12 UTC, Henri Yandell

Note You need to log in before you can comment on or make changes to this bug.
Description Henri Yandell 2011-07-26 06:12:43 UTC
Created attachment 27314 [details]
Patch to stop CR & LF in the header.

InternalInputBuffer allows newlines in http header field names. As an example:

Foo: Val1
Bar: Val2
Hup: Val3

This will lead to a field name of 'MISSHup' and not 'Hup'.

Digging into the specs, I think this goes back to RFC 822 which states:

     field-name  =  1*<any CHAR, excluding CTLs, SPACE, and ":">


     CTL         =  <any ASCII control           ; (  0- 37,  0.- 31.)
                     character and DEL>          ; (    177,     127.)

I think this is saying that field-name should not contain CR or LF.

I've attached a patch that stops CR & LF. Two improvements I could see, if there's agreement with this as a bug, are adding a unit test to TestInternalInputBuffer and changing the code so it disallows space and any other ctrl character in the field name.
Comment 1 Mark Thomas 2011-07-26 16:55:56 UTC
The exact quote from RFC 2616 is that HTTP headers "follow the same generic format as that given in Section 3.1 of RFC 822". RFC 2616 goes further in defining exactly what is permitted so RFC 2616 remains the relevant specification in this case.

As per RFC 2616, HTTP header names are tokens which mean no CTLs and no separators which requires further restrictions than no CTLs and no space.

The patch only addresses the HTTP BIO connector. The issue also needs to be addressed for the HTTP NIO and HTTP APR/native connectors.

I have an untested patch enforces the RFC 2616 requirements and drops the header line if an invalid header name is presented (that seemed a better option than returning a 400 response).

I'm currently running the test suite for all three connectors and will commit the patch assuming the tests pass.
Comment 2 Mark Thomas 2011-07-27 09:10:43 UTC
This has been fixed in 7.0.x and will be included in 7.0.20 onwards.
Comment 3 Christopher Schultz 2011-08-01 15:01:12 UTC
Mark, can you give a revision reference?

I'm interested in whether you or not you think response-splitting protection is worthwhile in other cases (i.e. header values).
Comment 4 Henri Yandell 2011-08-01 16:41:49 UTC