Opened 13 months ago

Closed 7 months ago

#9646 defect closed fixed (fixed)

twisted.web.http server should reject whitespace between header field-name and colon

Reported by: Tom Most Owned by: Tom Most
Priority: normal Milestone:
Component: web Keywords:
Cc: Branch: 9646-http-header-wsp-colon
branch-diff, diff-cov, branch-cov, buildbot
Author:

Description

RFC 7230:

   No whitespace is allowed between the header field-name and colon.  In
   the past, differences in the handling of such whitespace have led to
   security vulnerabilities in request routing and response handling.  A
   server MUST reject any received request message that contains
   whitespace between a header field-name and colon with a response code
   of 400 (Bad Request).  A proxy MUST remove any such whitespace from a
   response message before forwarding the message downstream.

Twisted does not reject such requests, but it should.

Change History (6)

comment:1 Changed 13 months ago by Tom Most

Branch: 9646-http-header-wsp-colon
Owner: set to Tom Most
Status: newassigned

comment:2 Changed 13 months ago by Tom Most

Keywords: review added
Owner: Tom Most deleted
Status: assignednew

PR here: https://github.com/twisted/twisted/pull/1151

(I am putting this in review before PR goes green because I may lose power again. PG&E is the worst. The worst.)

comment:3 Changed 12 months ago by hawkowl

Keywords: review removed
Owner: set to Tom Most

LGTM.

Only one nit: should we be checking for whitespace anywhere in the header key value, since it's not actually allowed it anyway? I don't think we check for that. But that's fine in a follow-up if you think it's something that matters.

Merge away.

comment:4 Changed 7 months ago by Wilfredo Sánchez Vega

yeah… I'd suggest tests to ensure these are all processed correctly:

  • " : bar"
  • "f o o: bar"
  • " foo: bar"

comment:5 Changed 7 months ago by Tom Most

Thank you for the review, hawkowl! I'm sorry I seem to have lost track of this PR, but better late than never, I suppose. I will go ahead and merge now.

I filed #9743 to reject HTTP headers with whitespace (thank you for the examples, Wilfredo). I do wonder if that would have compatibility concerns. This ticket was crafted to fall firmly in the "gross violation of specifications" category. I will have to refresh myself on the RFC and see if that applies.

comment:6 Changed 7 months ago by Tom Most <twm@…>

Resolution: fixed
Status: newclosed

In 08186a1f:

Merge pull request #1151 from twisted/9646-http-header-wsp-colon

Author: twm
Reviewer: hawkowl
Fixes: ticket:9646

t.w.http.HTTPChannel: Reject more invalid HTTP headers

Note: See TracTickets for help on using tickets.