sahvx655-wq opened a new pull request, #3604: URL: https://github.com/apache/brpc/pull/3604
### What problem does this PR solve? Issue Number: resolve Problem Summary: `RpcMeta.attachment_size` is `optional int32` (baidu_rpc_meta.proto) and is copied from the peer. In `ProcessRpcRequest` and `ProcessRpcResponse` (src/brpc/policy/baidu_rpc_protocol.cpp) it is validated only against an upper bound (`req_size < attachment_size`, `attachment_size > res_size`). A negative value passes both guards and reaches ``` int body_without_attachment_size = res_size - meta.attachment_size(); ``` With `attachment_size = INT32_MIN` and a small body (an empty body is a valid baidu_std message), `res_size - attachment_size` overflows signed `int`, which is undefined behavior; the UBSan job in CI aborts on it. For other negative values the subtraction yields a size larger than the payload, so the body/attachment split silently desyncs. Reproduced the arithmetic with the real types under `-fsanitize=undefined`: `attachment_size=INT32_MIN`, `req_size=0` gives `runtime error: signed integer overflow: 0 - -2147483648 cannot be represented in type 'int'` and aborts; with the negative value rejected it returns cleanly. Siblings checked: hulu uses `cutn(user_message_size)` (bounded by IOBuf, no subtraction) and sofa has no attachment, so neither shares the pattern. Both affected sites are in this file. ### What is changed and the side effects? Changed: - Reject `attachment_size < 0` at the request and response guards, so the later `int` subtraction always has a non-negative operand. Side effects: - Performance effects: none. - Breaking backward compatibility: none. A valid `attachment_size` is a byte count and is always `>= 0`; only malformed negative values are now rejected with the existing `SetFailed` path. --- ### Check List: - Changes are compilable; the diff only tightens an existing condition. - Verified with a UBSan reproducer of the exact subtraction (before: abort on `INT32_MIN`; after: clean reject). No committed unit test since this parser path needs a full server/client round-trip to exercise and I could not build the suite in my environment; happy to add one if you prefer. - Follows the Code of Conduct. This fix was found and prepared with AI assistance and reviewed by me before submitting. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
