New issue
Advanced search Search tips

Issue 627394 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

net::HttpUtil::IsValidHeaderValue() permits embedded newlines

Project Member Reported by ricea@chromium.org, Jul 12 2016

Issue description

IsValidHeaderValue() is used for validation of untrusted input. It should not be possible to inject new headers using a header value which passes this check.

The method only rejects an explicit CRNL in the header value. However, some web servers treat a single CR or NL as line terminators, even when other lines in the same request use CRNL. In particular, Apache 2.4.10 treats NL this way.

This means that input validated by this function can still inject new headers, at least for some servers.

This is related to the larger issue of RFC 7230 compliance covered by  issue 455099 , but is narrower in scope.
 

Comment 1 by ricea@chromium.org, Jul 12 2016

Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0723324a1ea7ee8b3fc71e61ed8767c045f9883

commit b0723324a1ea7ee8b3fc71e61ed8767c045f9883
Author: ricea <ricea@chromium.org>
Date: Wed Jul 13 03:53:44 2016

Reject line terminators in HttpUtil::IsValidHeaderValue()

IsValidHeaderValue() is used for validation of untrusted input in at
least two places:

https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/downloads/downloads_api.cc?q=file:%5Esrc/chrome/browser/extensions/api/downloads/downloads_api.cc+IsValidHeaderName
https://cs.chromium.org/chromium/src/content/browser/renderer_host/websocket_host.cc?q=file:%5Esrc/content/browser/renderer_host/websocket_host.cc+IsValidHeaderValue

Currently this method only rejects an explicit CRNL in the header
value. However, some web servers treat a single CR or NL as line
terminators, even when other lines in the same request use CRNL. In
particular, Apache 2.4.10 treats NL this way.

This means that input validated by this function can still inject new
headers, at least for some servers.

This CL modifies the function to reject individual CR and NL in the
header value. This is still not as strict as RFC 7230, but it is
sufficient to prevent the potential privilege escalation.

BUG= 627394 

Review-Url: https://codereview.chromium.org/2134083003
Cr-Commit-Position: refs/heads/master@{#404987}

[modify] https://crrev.com/b0723324a1ea7ee8b3fc71e61ed8767c045f9883/net/http/http_util.cc
[modify] https://crrev.com/b0723324a1ea7ee8b3fc71e61ed8767c045f9883/net/http/http_util_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 13 2016

Labels: merge-merged-2795
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0723324a1ea7ee8b3fc71e61ed8767c045f9883

commit b0723324a1ea7ee8b3fc71e61ed8767c045f9883
Author: ricea <ricea@chromium.org>
Date: Wed Jul 13 03:53:44 2016

Reject line terminators in HttpUtil::IsValidHeaderValue()

IsValidHeaderValue() is used for validation of untrusted input in at
least two places:

https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/downloads/downloads_api.cc?q=file:%5Esrc/chrome/browser/extensions/api/downloads/downloads_api.cc+IsValidHeaderName
https://cs.chromium.org/chromium/src/content/browser/renderer_host/websocket_host.cc?q=file:%5Esrc/content/browser/renderer_host/websocket_host.cc+IsValidHeaderValue

Currently this method only rejects an explicit CRNL in the header
value. However, some web servers treat a single CR or NL as line
terminators, even when other lines in the same request use CRNL. In
particular, Apache 2.4.10 treats NL this way.

This means that input validated by this function can still inject new
headers, at least for some servers.

This CL modifies the function to reject individual CR and NL in the
header value. This is still not as strict as RFC 7230, but it is
sufficient to prevent the potential privilege escalation.

BUG= 627394 

Review-Url: https://codereview.chromium.org/2134083003
Cr-Commit-Position: refs/heads/master@{#404987}

[modify] https://crrev.com/b0723324a1ea7ee8b3fc71e61ed8767c045f9883/net/http/http_util.cc
[modify] https://crrev.com/b0723324a1ea7ee8b3fc71e61ed8767c045f9883/net/http/http_util_unittest.cc

Comment 4 by ricea@chromium.org, Jul 19 2016

Status: Fixed (was: Assigned)
Components: Internals>Network
Components: -Internals>Network>HTTP

Sign in to add a comment