security revert CVE-2016-2216 didn't work with HPE_UNEXPECTED_CONTENT_LENGTH

Version: v4.4.0
Platform: Darwin MacBook-Pro.local 15.3.0 Darwin Kernel Version 15.3.0: Thu Dec 10 18:40:58 PST 2015; root:xnu-3248.30.4~1/RELEASE_X86_64 x86_64

We encounter this issue when we upgrade node.js from v4.2.x to v4.3.x or v4.4.0.

A service we depends on will return both Transfer-Encoding and Content-Length in headers, and the node.js > 4.3.x will throw a HPE_UNEXPECTED_CONTENT_LENGTH error when we make a request with the service. and security revert CVE-2016-2216 also can't resolve this problem.

Also, according to RFC 2616

If a Content-Length header field (section 14.13) is present, its
decimal value in OCTETs represents both the entity-length and the
transfer-length. The Content-Length header field MUST NOT be sent
if these two lengths are different (i.e., if a Transfer-Encoding header field is present). If a message is received with both a
Transfer-Encoding header field and a Content-Length header field,
the latter MUST be ignored.

So I think Node.js should ignore the content-length when both header was given rather than throw a error.

Here is some code the reproduce this issue

const http = require('http');

const server = http.createServer((req, res) => {
    res.setHeader('Transfer-Encoding', 'chunked');
    res.setHeader('Content-Length', 344);
    res.end('ok');
}).listen(8085);

const options = {
    port: 8085,
    hostname: '127.0.0.1',
    method: 'GET',
};

const req = http.request(options);
req.on('error', function (err) {
    console.error(err);
});
req.end();

➜  ~ node --security-revert=CVE-2016-2216 index.js
SECURITY WARNING: Reverting CVE-2016-2216: Strict HTTP Header Parsing
{ [Error: Parse Error] bytesParsed: 123, code: 'HPE_UNEXPECTED_CONTENT_LENGTH' }

/cc @nodejs/lts @nodejs/security

RFC 7230 obsoletes RFC 2616, and there is this statement:

If a message is received with both a Transfer-Encoding and a
       Content-Length header field, the Transfer-Encoding overrides the
       Content-Length.  Such a message might indicate an attempt to
       perform request smuggling (Section 9.5) or response splitting
       (Section 9.4) and ought to be handled as an error.  A sender MUST
       remove the received Content-Length field prior to forwarding such
       a message downstream.

Taking important parts out of it:

Receiving both Transfer-Encoding and Content-Length is an error case, and should be handled like this
Sending both Transfer-Encoding and Content-Length is a violation of protocol.

IMO, won't fix. Sorry!

I see..But still I think --security-revert=CVE-2016-2216 should be able to revert this break change in the original design.

@jasnell

I agree. cc @jasnell

I think what you're asking for is --security-revert=CVE-2016-2086 as this is a separate issue to allowable characters which is what CVE-2016-2216 covered.

I'm open to adding a revert for this CVE.
On Mar 17, 2016 9:32 PM, "Rod Vagg" notifications@github.com wrote:

I think what you're asking for is --security-revert=CVE-2016-2086 as this
is a separate issue to allowable characters which is what CVE-2016-2216
covered.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#5754 (comment)

@hefangshi

@hefangshi Has the duplicate header issue itself been reported to the service in question yet? They really shouldn't be sending these headers in the first place.

@joepie91

@joepie91 Sure I did, but I think this would be a common issue, so I posted here :)

Alright, fair enough, just wanted to check :)

I'll have to explore making this additional revert available. The
http-parser lib does not make it easy while maintaining ABI compatibility.
It's on my to-do list to make it easier but that'll require a more
significant change.
On Mar 23, 2016 6:42 PM, "Sven Slootweg" notifications@github.com wrote:

Alright, fair enough, just wanted to check :)

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#5754 (comment)

Closing due to lack of forward progress on this

Navigation Menu

Repository navigation

security revert CVE-2016-2216 didn't work with HPE_UNEXPECTED_CONTENT_LENGTH #5754

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

security revert CVE-2016-2216 didn't work with HPE_UNEXPECTED_CONTENT_LENGTH #5754

Description

Activity

MylesBorins commented on Mar 17, 2016

indutny commented on Mar 17, 2016

hefangshi commented on Mar 18, 2016

indutny commented on Mar 18, 2016

rvagg commented on Mar 18, 2016

jasnell commented on Mar 18, 2016

joepie91 commented on Mar 20, 2016

hefangshi commented on Mar 24, 2016

joepie91 commented on Mar 24, 2016

jasnell commented on Mar 24, 2016

jasnell commented on May 30, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions