From: Frederic Lecaille Date: Tue, 3 Nov 2026 06:47:54 +0000 (+0100) Subject: BUG/MINOR: sample: realign the ungrpc converter on message boundaries X-Git-Tag: v3.5-dev11~64 X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=3a319e14cff6bc43505cfae180da786e50947147;p=haproxy.git BUG/MINOR: sample: realign the ungrpc converter on message boundaries sample_conv_ungrpc() walks the GRPC messages of the body with two variables: , the read cursor, and , the bytes left. They are supposed to describe the same position. When protobuf_field_lookup() fails, it does not always leave at the end of the message. For a length-delimited field it stops on the value, after the key and the length. But the loop still subtracts the whole message length from , without moving . The cursor and the counter then describe different parts of the input, and the next iteration reads a "gRPC header" from the previous message's payload. A 23-byte body shows it: 00 0000000a 0a 08 00 00 00 00 03 08 b9 0a 00 00000003 08 8f 4e ungrpc(1,int64) returns 1337, which is the value of the first message's field. The second message holds the real field 1 = 9999 but is never reached. The counter can also over-count, and skip real messages. Save the end of the message rigght after the "grpc_left < left" check, then set to it when the lookup fails, so the next iteration starts on a real message boundary. Nothing changes on the success path, nor for fully parseable bodies, where the lookup already stops at the message end. The multi-message walk is intended for streaming bodies and is kept. This is not a memory-safety issue: and the counter advance by the header together, and the lookup moves forward by at most the message length, which is the same amount removed from the counter. This is also not a policy bypass like a4f5eea841 ("BUG/MEDIUM: protobuf: fix nested path bypass in field lookup"). There, a flat sibling field could satisfy a nested path that the configuration explicitly restricted, so the client gained a capability it did not have. Here the client writes the whole body and can already obtain any value with a single well-formed message. The bug only makes the proxy read a value that is the field of no message of the body, while the backend reads another one. Hence MINOR and not MEDIUM. The loop was written this way when the converter was added in 2.0 (1fceee8316) and did not change since 5f33f85ce8, apart from the unaligned access fix. Not a regression. Backport to 2.0 and above. Thank you to Owais Lone for having reported this bug and provided this patch. Must be backported to all stable versions. Reported-by: Owais Lone --- diff --git a/src/sample.c b/src/sample.c index ba50695df..e5f7343f9 100644 --- a/src/sample.c +++ b/src/sample.c @@ -4402,6 +4402,7 @@ static int sample_conv_ungrpc(const struct arg *arg_p, struct sample *smp, void while (grpc_left > GRPC_MSG_HEADER_SZ) { size_t grpc_msg_len, left; + unsigned char *msg_end; grpc_msg_len = left = ntohl(read_u32(pos + GRPC_MSG_COMPRESS_FLAG_SZ)); @@ -4411,9 +4412,19 @@ static int sample_conv_ungrpc(const struct arg *arg_p, struct sample *smp, void if (grpc_left < left) return 0; + /* protobuf_field_lookup() walks the message and advances , + * but on failure it leaves it wherever it stopped, for instance + * on the value of a length-delimited field whose type does not + * match, or at the end of a narrowed sub-message. The next + * message must be decoded from the end of the current one, so + * remember that position rather than relying on the lookup. + */ + msg_end = pos + grpc_msg_len; + if (protobuf_field_lookup(arg_p, smp, &pos, &left)) return 1; + pos = msg_end; grpc_left -= grpc_msg_len; }