BUG/MINOR: httpclient/lua: fix stack overread when copying the headers

hlua_httpclient_table_to_hdrs() collects the header values of a Lua
httpclient request into a stack array of tune.http.maxhdr entries, then
copies hdr_num + 1 entries into the allocated result, although the
array never contains a terminator: the result is allocated with calloc()
and its last entry is set to IST_NULL right after the copy.

Since commit 2db801c63 ("BUG/MINOR: hlua: fix stack overflow in
httpclient headers conversion") hdr_num may reach tune.http.maxhdr, and
the copy then reads one entry past the end of the array. E.g. with the
default value:

    local v = {}
    for i = 1, 101 do v[i] = "v" .. i end
    hc:get{ url = "http://127.0.0.1:8080/", headers = { ["x-many"] = v } }

ASan reports a dynamic-stack-buffer-overflow READ of size 3264 (102
entries) in hlua_httpclient_table_to_hdrs(). With fewer values, the
extra entry is uninitialized stack, which is then overwritten.

Let's only copy the hdr_num collected entries.

This should be backported to all stable versions with the Lua
httpclient (2.5 and above), on top of 2db801c63 where it was
backported. Before 3.5 the function is in src/hlua.c.

diff --git a/src/http_client.c b/src/http_client.c
index 3f0c0a3d408ceb4e961709f2471425e9b29c3c24..c5b1a0a7397920cdc092efa16d2699640fe93e0d 100644 (file)
--- a/src/http_client.c
+++ b/src/http_client.c
@@ -1745,7 +1745,7 @@ static struct http_hdr *hlua_httpclient_table_to_hdrs(lua_State *L)
                result = calloc((hdr_num + 1), sizeof(*result));
                if (!result)
                        goto skip_headers;
-               memcpy(result, hdrs, sizeof(struct http_hdr) * (hdr_num + 1));
+               memcpy(result, hdrs, sizeof(struct http_hdr) * hdr_num);
 
                result[hdr_num].n = IST_NULL;
                result[hdr_num].v = IST_NULL;