Summary: | log overflows with a long request | ||
---|---|---|---|
Product: | Apache httpd-2 | Reporter: | Tsurutani Naoki <turutani> |
Component: | Core | Assignee: | Apache HTTPD Bugs Mailing List <bugs> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | vhoff |
Priority: | P3 | ||
Version: | 2.0.49 | ||
Target Milestone: | --- | ||
Hardware: | Other | ||
OS: | other |
Description
Tsurutani Naoki
2004-04-14 09:54:39 UTC
A fix is in apache 2.1-dev and proposed for merge into stable branch. See http://cvs.apache.org/viewcvs.cgi/httpd-2.0/server/protocol.c?r1=1.147&r2=1.148 Thank you for your fix. But I think another fixs are necessary. --- protocol.c.orig Fri May 7 15:37:03 2004 +++ protocol.c Fri May 7 15:37:47 2004 @@ -249,7 +249,7 @@ } /* Would this overrun our buffer? If so, we'll die. */ - if (n < bytes_handled + len) { + if (n <= bytes_handled + len) { *read = bytes_handled; if (*s) { /* ensure this string is terminated */ @@ -387,7 +387,7 @@ if (c == APR_ASCII_BLANK || c == APR_ASCII_TAB) { /* Do we have enough space? We may be full now. */ if (bytes_handled >= n) { - *read = n; + *read = n-1; /* ensure this string is terminated */ (*s)[n-1] = '\0'; return APR_ENOSPC; --(end of diffs)-- About previous patch, I think that in the first section the "if-else" section is not necessary if "len" is not negative. And I think, while additional string of "HTTP/1.0" is attatched to tail of the request field in the logfile if uri is too long and is truncated for logging, it is not natural that there are no space between original request recoreded in logfile and additional "HTTP/1.0". I propose to change "HTTP/1.0" to " HTTP/1.0" in line 604 of httpd-2.0/server/protocol.c rev1.148 (I do not know about the side-effects by this change). *** Bug 29425 has been marked as a duplicate of this bug. *** I'm running Apache 2.0.52 and I've been getting this problem quite a lot. It looks more like random attempts to cause buffer overflows to me, as the requests are coming from unexpected sources (not that many people know about my webserver and I know when the ones that do connect). However I am using No- IP.com for dynamic DNS redirection. From what I could read in the bug activity it looks like this was meant to be fixed to stop a buffer overflow exploit, however I don't think it has been fixed. I am running Apache on Windows XP SP1 behind an ADSL router and I am also running a firewall, but I can't stop attacks like these. I am looking at moving my server to a FreeBSD box, but I don't know if that will fix the problem. This might be a good moment to mention bug 29449 once again. It provides a simple mechanism for administrators to limit the log pollution resulting from long requests. The original bug was caused by forgetten NULL termination of the string, and the too-long-request was not essential. Restriction of request is a good idea, I think, but is a different problem. As per comments above this is fixed in current releases. I don't agree with either of the changes in comment 2: - if (n < bytes_handled + len) { + if (n <= bytes_handled + len) { this means that exactly N bytes cannot be stored in the buffer, only N - 1; why is that necessary? - *read = n; + *read = n-1; /* ensure this string is terminated */ (*s)[n-1] = '\0'; since *read is suppoed to be set to the total length of the string *including* the NUL terminator, that isn't correct either. I'm going to mark this closed and suggest opening new a new issue for a new bug. |