Fix for CVE-2018-12122 on Node 6.15.0 does not reset headersTimeout on keep-alive requests

[pracucci]

• Version: v6.15.0
• Platform: Darwin Marco-13-2017.local 17.5.0 Darwin Kernel Version 17.5.0: Mon Mar 5 22:24:32 PST 2018; root:xnu-4570.51.1~1/RELEASE_X86_64 x86_64
• Subsystem: http

The fix for CVE-2018-12122 in Node 6.15.0 looks is not resetting the headersTimeout clock once the full request headers have been received and this cause the socket to be destroyed after headersTimeout in a keep-alive connection.

I'm not familiar with node sources but looking at the commit by @mcollina to backport the fix to node 6.15.0 I've the feeling that parser.parsingHeadersStart is never set to 0 once the request headers have been received.

However, looking at the commit to fix the same issue in node 8.14.0, the parsingHeadersStart is reset to zero in parserOnIncoming().

How to reproduce the issue

1. Create an http server with node 6.15.0 and lower headersTimeout to get a faster test

const http = require("http");

const server = http.createServer((req, res) => {
    res.writeHead(200);
    res.end();
});

server.headersTimeout = 10000;
server.keepAliveTimeout = 60000;

server.listen(4050);

2. Connect to the server with telnet localhost 4050

3. Send the first request

GET / HTTP/1.1
Connection: keep-alive

4. Wait a bit more than 10 seconds and then send a second request

GET / HTTP/1.1
Connection: keep-alive

The connection will be closed right after sending the first line of the second HTTP request

[hohokus]

I am seeing exactly the same. Originally appeared as an intermittent "502 Proxy Error" (we have Apache sitting in front of Node), but your telnet reproduce works 100%. Thanks!

[pracucci]

The PR #24796 - landed in Node 6.15.1 - fixes this. Thanks @mcollina !