bug: segmentation fault due to invalid cast

[2EXP]

Description

functions sharing str_buf as buffer may cause ngx_http_wasm_req_get_headers segmentation fault because of the invalid cast between ngx_str_t.len (type size_t) and ngx_http_lua_ffi_str_t.len (type int).

Here, get_req_headers is resolved to ngx_http_lua_ffi_req_get_headers, but their parameter types proxy_wasm_table_elt_t and ngx_http_lua_ffi_table_elt_t have different structure.

https://github.com/api7/wasm-nginx-module/blob/e6a80099d7a08f829c50bf3e2bb7cc539f03d244/src/http/ngx_http_wasm_api.c#L136

On 64bit OS, it will read len (type size_t) extra 4 dirty bytes which may be previously written by another function sharing str_buf.

Finally, ngx_memcpy with the wrong len makes the segmentation fault.

https://github.com/api7/wasm-nginx-module/blob/e6a80099d7a08f829c50bf3e2bb7cc539f03d244/src/http/ngx_http_wasm_api.c#L978

Data Structure Detail Comparison

https://github.com/api7/wasm-nginx-module/blob/e6a80099d7a08f829c50bf3e2bb7cc539f03d244/src/http/ngx_http_wasm_api.c#L93-L94

lua-nginx-module - ngx_http_lua_ffi_req_get_headers

https://github.com/openresty/lua-nginx-module/blob/653d6a36f46b077cb902d7ba40824c299cf9bbf4/src/ngx_http_lua_headers.c#L810-L812

lua-nginx-module cast len from original type size_t to int. https://github.com/openresty/lua-nginx-module/blob/653d6a36f46b077cb902d7ba40824c299cf9bbf4/src/ngx_http_lua_headers.c#L845

---

https://github.com/api7/wasm-nginx-module/blob/a418cc3861e218b6425b6eccec411a6394eedd2a/src/proxy_wasm/proxy_wasm_types.h#L24-L27

lua-nginx-module - ngx_http_lua_ffi_table_elt_t

https://github.com/openresty/lua-nginx-module/blob/9b7bde2e7986d2150606e33e303c6bd1c40271a6/src/ngx_http_lua_util.h#L106-L109

---

nginx - ngx_str_t

https://github.com/nginx/nginx/blob/a64190933e06758d50eea926e6a55974645096fd/src/core/ngx_string.h#L16-L19

lua-nginx-module - ngx_http_lua_ffi_str_t

https://github.com/openresty/lua-nginx-module/blob/4bbb57aa4a6995bd3efaf35c01e826de6f3cdf3a/src/api/ngx_http_lua_api.h#L37-L41

[spacewander]

Thanks for your detailed analysis. Would you like to submit a bugfix for it? Maybe we can just change the type in proxy_wasm_table_elt_t to match ngx_http_lua_ffi_table_elt_t?

[2EXP]

ngx_log_debug2 %V format specifier relies on ngx_str_t to output. The wrong len also causes the wrong debug log.

https://github.com/api7/wasm-nginx-module/blob/e6a80099d7a08f829c50bf3e2bb7cc539f03d244/src/http/ngx_http_wasm_api.c#L981-L983

---

values in ngx_http_wasm_resp_get_header has the similar problem. The int32_t cast ofngx_http_wasm_copy_to_wasm corrects the wrong len of values, but future code accessing values->len before the function call may cause errors.

https://github.com/api7/wasm-nginx-module/blob/e6a80099d7a08f829c50bf3e2bb7cc539f03d244/src/http/ngx_http_wasm_api.c#L1344

https://github.com/api7/wasm-nginx-module/blob/e6a80099d7a08f829c50bf3e2bb7cc539f03d244/src/http/ngx_http_wasm_api.c#L1365

[2EXP]

Thanks for your detailed analysis. Would you like to submit a bugfix for it? Maybe we can just change the type in proxy_wasm_table_elt_t to match ngx_http_lua_ffi_table_elt_t?

I would, but I may need to figure out the impact of the changes. Only changing the type in proxy_wasm_table_elt_t won't fix the wrong ngx_log_debug2 debug log.

[2EXP]

I'm thinking about the following solutions. Any suggestions?

• change proxy_wasm_table_elt_t to use ngx_http_lua_ffi_str_t, add macro ngx_str_clear_len_high_bit for the debug log
• retain ngx_str_t, get header info directly from ngx_http_request_t
• clear str_buf before using

[spacewander]

I will vote for the first solution, which requires less change and less data modification. As the debug log, we can switch to use "%*s", size, data format instead of %V.