Hi Claudio,
This prevents undeadly.org from crashing when I send it the query I
got from Florian. Thanks a lot!
Paul
On Thu, Nov 11, 2021 at 04:41:21PM +0100, Claudio Jeker wrote:
| On Thu, Nov 11, 2021 at 04:29:58PM +0100, Otto Moerbeek wrote:
| > On Thu, Nov 11, 2021 at 04:13:37PM +0100, Florian Obser wrote:
| >
| > >
| > > No idea how to reproduce this, I'm just running an httpd with debug
| > > symbols and kern.nosuidcoredump=3
| > > Pretty sure this is the crash various people mumbled about.
| > >
| > > Smells like a use-after-fruit to me.
| >
| > In server_http.c:351 desc->http_query is set to point in the middle of
| > a string. In the cases of goto fail belows that it will not be
| > strdupped. A free of desc->http_query then later bombs.
| >
|
| I debugged something like this with tobhe@ the other day. I remember I
| mentioned to benno@ to look out for this dumb pattern of first assigning a
| variable and then doing variable = strdup(variable). He fixed one case but
| forgot the other.
|
| I think all this http_query handling can be moved down and while at it use
| a local variable as well.
|
| --
| :wq Claudio
|
| Index: server_http.c
| ===================================================================
| RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
| retrieving revision 1.148
| diff -u -p -r1.148 server_http.c
| --- server_http.c 5 Nov 2021 19:01:02 -0000 1.148
| +++ server_http.c 11 Nov 2021 15:36:41 -0000
| @@ -228,7 +228,7 @@ server_read_http(struct bufferevent *bev
| struct evbuffer *src = EVBUFFER_INPUT(bev);
| char *line = NULL, *key, *value;
| const char *errstr;
| - char *http_version;
| + char *http_version, *query;
| size_t size, linelen;
| int version;
| struct kv *hdr = NULL;
| @@ -348,9 +348,6 @@ server_read_http(struct bufferevent *bev
| }
|
| *http_version++ = '\0';
| - desc->http_query = strchr(desc->http_path, '?');
| - if (desc->http_query != NULL)
| - *desc->http_query++ = '\0';
|
| /*
| * We have to allocate the strings because they could
| @@ -378,10 +375,13 @@ server_read_http(struct bufferevent *bev
| goto fail;
| }
|
| - if (desc->http_query != NULL &&
| - (desc->http_query =
| - strdup(desc->http_query)) == NULL)
| - goto fail;
| + query = strchr(desc->http_path, '?');
| + if (query != NULL) {
| + *query++ = '\0';
| +
| + if ((desc->http_query = strdup(query)) == NULL)
| + goto fail;
| + }
|
| } else if (desc->http_method != HTTP_METHOD_NONE &&
| strcasecmp("Content-Length", key) == 0) {
|
--
>++++++++[<++++++++++>-]<+++++++.>+++[<------>-]<.>+++[<+
+++++++++++>-]<.>++[<------------>-]<+.--------------.[-]
http://www.weirdnet.nl/
