On Tue, Jan 06, 2015 at 02:01:52PM +0000, Philip Martin wrote:
> The crash is happening in the code that parses the status line,
> i.e. when handling something like
>
> HTTP/1.1 200 OK
>
> or
>
> HTTP/1.1 207 Multi-Status
>
> or
>
> HTTP/1.1 401 Authorization Required
> Breakpoint 1, parse_status_line (ctx=0x46b758, allocator=0x4623e0)
> at buckets/response_buckets.c:148
> 148 ctx->sl.reason = serf_bstrmemdup(allocator, reason,
> (gdb) l
> 143 if (apr_isspace(*reason)) {
> 144 reason++;
> 145 }
> 146
> 147 /* Copy the reason value out of the line buffer. */
> 148 ctx->sl.reason = serf_bstrmemdup(allocator, reason,
> 149 ctx->linebuf.used
> 150 - (reason - ctx->linebuf.line));
> 151
> 152 return APR_SUCCESS;
> (gdb) p ctx->linebuf.used
> $8 = 15
> (gdb) x/15c ctx->linebuf.line
> 0x46b788: 72 'H' 84 'T' 84 'T' 80 'P' 47 '/' 49 '1' 46 '.' 49 '1'
> 0x46b790: 32 ' ' 50 '2' 48 '0' 48 '0' 32 ' ' 79 'O' 75 'K'
> (gdb) p reason
> $9 = 0x46b795 "OKext/html; charset=iso-8859-1ry\"OpenSSL/1.0.1e DAV"
Note that this code fails to check for errors from apr_strtoi64().
The bytes beyond the status code number aren't verified but apr_strtoi64()
will try to parse them and perhaps fail.
This patch against serf trunk adds error checking.
It may not fix the segfault problem, though.
Index: buckets/response_buckets.c
===================================================================
--- buckets/response_buckets.c (revision 2445)
+++ buckets/response_buckets.c (working copy)
@@ -140,6 +140,8 @@ static apr_status_t parse_status_line(response_con
ctx->sl.version = SERF_HTTP_VERSION(ctx->linebuf.line[5] - '0',
ctx->linebuf.line[7] - '0');
ctx->sl.code = apr_strtoi64(ctx->linebuf.line + 8, &reason, 10);
+ if (errno == ERANGE || reason == ctx->linebuf.line + 8)
+ return SERF_ERROR_BAD_HTTP_RESPONSE;
/* Skip leading spaces for the reason string. */
if (apr_isspace(*reason)) {
