membphis commented on PR #13483: URL: https://github.com/apache/apisix/pull/13483#issuecomment-4665990166
I think this still leaves one malformed RESP path unsafe. The new validation protects the array argument count (`*<n>`), but `read_len()` is also used for bulk-string lengths. Since the new pattern still accepts negative decimal tokens (`^-?%d+$`), a request such as: ```text *1\r\n$-1\r\n ``` can return `n = -1` for the command bulk length. `read_req()` then reaches `sk:read(n + 2)` and later `ffi_str(p, n)` without rejecting `n < 0`. Locally, LuaJIT reports `ffi.string(ptr, -1)` as `string length overflow`, so a malformed client request can still hit an uncaught runtime-error path. Suggested fix: reject negative bulk-string lengths before every bulk `sk:read(n + 2)` in `read_req()` (both command and argument lengths), and add regression tests for negative command bulk length and negative argument bulk length. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
