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

RESOLVED FIXED in Firefox 65

Status

()

defect
P2
normal
RESOLVED FIXED
8 months ago
7 months ago

People

(Reporter: annevk, Assigned: bagder)

Tracking

unspecified
mozilla65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

(Whiteboard: [necko-triaged] )

Attachments

(1 attachment)

Reporter

Comment 1

8 months ago
(In general 0x0C shouldn't be considered whitespace when parsing HTTP header values. Not sure yet to what extent this problem is widespread.)
Dragana -- can you have a look and let me know what priority you think this is? Should we tackle this soon, or next quarter?
Flags: needinfo?(dd.mozilla)
Whiteboard: [necko-triaged]
Assignee

Comment 3

7 months ago
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")) {
Assignee

Comment 4

7 months ago
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;
Sounds like we have a volunteer.
Assignee: nobody → daniel
Whiteboard: [necko-triaged] → [necko-triaged]
Priority: -- → P2
Assignee

Comment 6

7 months ago
The nsCSstring's StripWhitespace() method is not very HTTP "friendly".
Assignee

Updated

7 months ago
Blocks: 1505752

Comment 7

7 months ago
Pushed by daniel@haxx.se:
https://hg.mozilla.org/integration/autoland/rev/c9f524cff7df
do not strip 0x0c from X-Content-Type-Options header fields r=dragana

Comment 8

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c9f524cff7df
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Flags: needinfo?(dd.mozilla)
You need to log in before you can comment on or make changes to this bug.