1503517 - 0x0C is not whitespace for X-Content-Type-Options

Status: RESOLVED FIXED

(Core :: DOM: Networking, defect, P2)

Description

[Anne (:annevk)]

See https://github.com/web-platform-tests/wpt/pull/13559 for tests and https://github.com/whatwg/fetch/pull/818 for the new parser definition.

Comment 1

[Anne (:annevk)]

(In general 0x0C shouldn't be considered whitespace when parsing HTTP header values. Not sure yet to what extent this problem is widespread.)

Comment 2

[Selena Deckelmann :selenamarie :selena]

Dragana -- can you have a look and let me know what priority you think this is? Should we tackle this soon, or next quarter?

Comment 3

[Daniel Stenberg [:bagder]]

Maybe just converting them to space before stripping them?

diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
index f0c86a3ab00e..8a86f1a1ebcd 100644
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -1471,10 +1471,11 @@ ProcessXCTO(nsIURI* aURI, nsHttpResponseHead* aResponseHead, nsILoadInfo* aLoadI
     if (idx > 0) {
       contentTypeOptionsHeader = Substring(contentTypeOptionsHeader, 0, idx);
     }
     // b) let's trim all surrounding whitespace
     //    e.g. "   NoSniFF   " -> "NoSniFF"
+    contentTypeOptionsHeader.ReplaceChar('\x0c', ' ');
     contentTypeOptionsHeader.StripWhitespace();
     // c) let's compare the header (ignoring case)
     //    e.g. "NoSniFF" -> "nosniff"
     //    if it's not 'nosniff' then there is nothing to do here
     if (!contentTypeOptionsHeader.EqualsIgnoreCase("nosniff")) {

Comment 4

[Daniel Stenberg [:bagder]]

I chatted with :annevk on IRC and I had the logic turned the wrong way. StripWhitespace() strips too much. Here's a take instead uses TrimHTTPWhitespace(). I have not tested this, only verified that it builds!

diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
index 8877037af95a..7414c97435e7 100644
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -1481,11 +1481,11 @@ ProcessXCTO(nsIURI* aURI, nsHttpResponseHead* aResponseHead, nsILoadInfo* aLoadI
     if (idx > 0) {
       contentTypeOptionsHeader = Substring(contentTypeOptionsHeader, 0, idx);
     }
     // b) let's trim all surrounding whitespace
     //    e.g. "   NoSniFF   " -> "NoSniFF"
-    contentTypeOptionsHeader.StripWhitespace();
+    nsHttp::TrimHTTPWhitespace(contentTypeOptionsHeader, contentTypeOptionsHeader);
     // c) let's compare the header (ignoring case)
     //    e.g. "NoSniFF" -> "nosniff"
     //    if it's not 'nosniff' then there is nothing to do here
     if (!contentTypeOptionsHeader.EqualsIgnoreCase("nosniff")) {
         // since we are getting here, the XCTO header was sent;

Comment 5

[Selena Deckelmann :selenamarie :selena]

Sounds like we have a volunteer.

Comment 6

[Daniel Stenberg [:bagder]]

Attachment: bug 1503517 - do not strip 0x0c from X-Content-Type-Options header fields

The nsCSstring's StripWhitespace() method is not very HTTP "friendly".

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9f524cff7df
do not strip 0x0c from X-Content-Type-Options header fields r=dragana

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/c9f524cff7df