Copilot commented on code in PR #3577:
URL: https://github.com/apache/brpc/pull/3577#discussion_r4162323952
##########
src/brpc/grpc.cpp:
##########
@@ -186,6 +186,14 @@ int64_t ConvertGrpcTimeoutToUS(const std::string*
grpc_timeout) {
if ((size_t)(endptr - grpc_timeout->data()) != grpc_timeout->size() - 1) {
return -1;
}
+ // The gRPC-over-HTTP2 spec restricts TimeoutValue to a positive integer of
+ // at most 8 digits. A peer that sends more digits (or a
negative/overflowed
+ // value that strtol clamped to LONG_MIN/MAX) would otherwise make the
+ // `timeout_value * <unit>` below overflow int64, which is undefined and
+ // feeds a bogus deadline into gettimeofday_us() on the request path.
+ if (timeout_value < 0 || timeout_value > 99999999) {
Review Comment:
This numeric bound does not enforce the stated eight-digit limit: for
example, `000000000S` has nine digits but `strtol` returns 0, so it still
passes this check. Validate the lexical `TimeoutValue` digit count before
conversion (while deciding whether to preserve the existing leading-`+`
compatibility), or narrow the comments and PR scope if only arithmetic-overflow
prevention is intended.
--
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]
