Project

General

Profile

Actions
Copy link

Bug #2670

closed

connection hangs when final cr/lf is in seperate packet

Added by mikevs about 9 years ago. Updated about 9 years ago.

Status:
Fixed
Priority:
High
Category:
core
Target version:
1.4.38
ASK QUESTIONS IN Forums:

Description

Starting from version 1.4.36, if you send a request without a body (GET, OPTIONS) and the headers and the final crlf are in different TCP packets, the connection hangs.

(I discovered this because I was using Apache as a reverse proxy in front of lighttpd and it behaves this way - at least with OPTIONS).

This does not happen if the size of the headers is smaller than approximately 64 bytes.

To reproduce:

telnet to a lighttpd server. Cut and paste the following data (WITHOUT an empty line at the end):

GET / HTTP/1.1
Host: localhost
X-Filler: 0123456789012345678901234567890

Now wait a short while and send the final CRLF (by pressing enter).

Expected result: HTTP reply
Actual result: connection hangs

This works on 1.4.35, and is broken on 1.4.36 and 1.4.37.

Reproduced on FreeBSD 10 and Ubuntu 15.04.

Configuration file used:

# Minimal lighttpd.conf
var.log_root    = "/tmp" 
var.server_root = "/tmp" 
var.state_dir   = "/tmp" 
var.home_dir    = "/tmp" 
var.conf_dir    = "/tmp" 

server.port = 1337
server.document-root = "/tmp"

Related issues 2 (0 open — 2 closed)

Has duplicate Bug #2686: Bug in read parsing codeDuplicate2015-12-01Actions
Has duplicate Bug #2683: HTTP requests with long GET are getting droppedDuplicateActions
Actions
Copy link
 #1

Updated by stbuehler about 9 years ago

  • Priority changed from Normal to High

Good catch - thanks for reporting.

Actions
Copy link
 #2

Updated by stbuehler about 9 years ago

The bug is old (in connections.c connection_handle_read_state()), but recent changes made it occur more often; the following patch fixes that.

Right now the bug triggers at the 64-byte boundary, the patch should make it occur somewhere at 4k or more.

diff --git a/src/connections.c b/src/connections.c
index 7e205af..46e46b1 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -336,10 +336,11 @@ static int connection_handle_read(server *srv, connection *con) {
     len = recv(con->fd, mem, mem_len, 0);
 #else /* __WIN32 */
     if (ioctl(con->fd, FIONREAD, &toread) || toread == 0 || toread <= 4*1024) {
-        if (toread > MAX_READ_LIMIT) toread = MAX_READ_LIMIT;
-    } else {
         toread = 4096;
     }
+    else if (toread > MAX_READ_LIMIT) {
+        toread = MAX_READ_LIMIT;
+    }
     chunkqueue_get_memory(con->read_queue, &mem, &mem_len, 0, toread);

     len = read(con->fd, mem, mem_len);
Actions
Copy link
 #3

Updated by stbuehler about 9 years ago

r2979 introduced the bug; the local code looks like it could end up triggering an out-of-bounds read, but I couldn't find a way to trigger it; I think the allocation pattern prevents it: we only search for at most 3 bytes in the next buffer, and the previous buffer would have to be smaller than that, in which case we would have reused the previous buffer for the following data.

Actions
Copy link
 #4

Updated by stbuehler about 9 years ago

  • Status changed from New to Fixed
  • % Done changed from 0 to 100

Applied in changeset r3043.

Actions
Copy link
 #5

Updated by stbuehler almost 9 years ago

  • Has duplicate Bug #2686: Bug in read parsing code added
Actions
Copy link
 #6

Updated by stbuehler almost 9 years ago

  • Has duplicate Bug #2683: HTTP requests with long GET are getting dropped added
Actions
Copy link

Also available in: Atom