Bug #1154
closedSegmentation fault in mod_proxy_backend_http.c
Description
When testing 15.0-svn r1820,we got Segmentation fault,after gdb digging,we found that in mod_proxy_backend_http.c line 196:
http://trac.lighttpd.net/trac/browser/trunk/src/mod_proxy_backend_http.c?rev=1820
case HTTP_CHUNK_END:
/* discard CRLF.*/
for(ch = 0; (size_t)(c->offset) < (c->mem->used - 1) && ch != '\n' ;) {
ch = c->mem->ptr[c->offset];
c->offset++;
in->bytes_out++;
}
c->mem->used should never be less then one,but it's so Segmentation fault happend.change the code to follow can fix it:
case HTTP_CHUNK_END:
/* discard CRLF.*/
for(ch = 0; c->mem->used >0 && (size_t)(c->offset) < (c->mem->used - 1) && ch != '\n' ;) {
ch = c->mem->ptr[c->offset];
c->offset++;
in->bytes_out++;
}
and more,base on http://trac.lighttpd.net/trac/changeset/1622 , maybe the other c->mem->used should be fix too.
Files
Updated by snailfly almost 17 years ago
Replying to snailfly:
c->mem->used should never be less then one,but it's so Segmentation fault happend.change the code to follow can fix it:
more in line 177:
if (c->offset == 0 && we_want == we_have) {
/* we are copying the whole buffer, just steal it */
chunkqueue_steal_chunk(out, c);
} else {
b = chunkqueue_get_append_buffer(out);
buffer_copy_string_len(b, c->mem->ptr + c->offset, we_want);
c->offset += we_want;
}
chunkqueue_steal_chunk change the value of c->mem-used to zero in some case.
mod_proxy_backend_http.c.185: (trace) befor chunkqueue_steal_chunk c->mem->used=5793 mod_proxy_backend_http.c.187: (trace) after chunkqueue_steal_chunk c->mem->used=0
chunkqueue_steal_chunk is found in chunk.c.
Updated by davidb54 over 15 years ago
- File lighttpd_bug1154.patch lighttpd_bug1154.patch added
- Status changed from New to Patch Pending
I believe I have also experienced this bug. I was able to reproduce it while running lighttpd under valgrind. The valgrind output that indicates the source of the crash is included below.
The problem seems to arise in some cases where an entire lighttpd memory chunk is moved from the input queue to the output queue. It is possible for this lighttpd chunk transfer to bring the HTTP chunk parser to the HTTP_CHUNK_END state with an empty chunk (because of the chunkqueue_steal_chunk call) via switch/case fall-through. The HTTP_CHUNK_END case does not check for an empty chunk, and can thus cause a crash by dereferencing a NULL pointer (as indicated by the valgrind output).
It looks to me like the solution is to use the approach suggested by snailfly in the HTTP_CHUNK_END case only (check that c->mem->used > 0), and avoid reaching for another chunk if we are in the HTTP_CHUNK_END state with an empty chunk (which is only possible if we fell through from HTTP_CHUNK_DATA after reading all of the bytes that we were looking for). This allows the HTTP chunk parser to finish the request instead of marking it as "ran out of data".
I have attached a patch.
-dave
Valgrind output ============================================================================
32015 Invalid read of size 132015 at 0x64B36C3: proxy_http_stream_decoder (mod_proxy_backend_http.c:203)
32015 by 0x62A8E4F: proxy_stream_encode_decode (mod_proxy_core.c:847)
32015 by 0x62AABD6: proxy_state_engine (mod_proxy_core.c:1708)
32015 by 0x62AB3B7: mod_proxy_core_start_backend (mod_proxy_core.c:2395)
32015 by 0x41C0DE: plugins_call_handle_read_response_content (plugin.c:387)
32015 by 0x40DEDF: connection_state_machine (connections.c:1313)
32015 by 0x4091EB: lighty_mainloop (server.c:1005)
32015 by 0x40AC46: main (server.c:1773)
32015 Address 0x0 is not stack'd, malloc'd or (recently) free'd
Updated by icy over 15 years ago
- Assignee changed from jan to stbuehler
- Pending set to Yes
- Patch available set to Yes
Updated by stbuehler about 15 years ago
- Pending changed from Yes to No
- Patch available changed from Yes to No
I don't see why don't want to get the next chunk if you stole the previous; that way you just behave like you already found CRLF, and i don't think that is a good idea.
Updated by stbuehler about 15 years ago
- Status changed from Patch Pending to Fixed
- % Done changed from 0 to 100
Applied in changeset r2443.
Also available in: Atom