From: Alan T. DeKok <aland@freeradius.org>
Date: Mon, 13 Feb 2023 20:41:06 +0000 (-0500)
Subject: be clearer about checking argc for overflows.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a6328e9ff7b01940d01abc2844a878ea57cf26cc;p=thirdparty%2Ffreeradius-server.git

be clearer about checking argc for overflows.

Should fix the fuzzer issues.
---

diff --git a/src/protocols/tacacs/decode.c b/src/protocols/tacacs/decode.c
index 678e4ecc8e0..c3b39159962 100644
--- a/src/protocols/tacacs/decode.c
+++ b/src/protocols/tacacs/decode.c
@@ -134,12 +134,11 @@ int fr_tacacs_packet_to_code(fr_tacacs_packet_t const *pkt)
 } while (0)
 
 #define ARG_COUNT_CHECK(_msg, _hdr) do { \
-	if ((p + _hdr.arg_cnt) > end) { \
+	fr_assert(p == (uint8_t const *) &(_hdr)); \
+	if ((p + data_len) > end) { \
 		fr_strerror_printf("Argument count %u overflows the remaining data (%zu) in the %s packet", _hdr.arg_cnt, end - p, _msg); \
 		goto fail; \
 	} \
-	p += _hdr.arg_cnt; \
-	data_len = 0; \
 	for (int i = 0; i < _hdr.arg_cnt; i++) { \
 		data_len += _hdr.arg_len[i]; \
 		if (data_len > (size_t) (end - p)) { \
@@ -764,7 +763,6 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu
 			if (data_len > (size_t) (end - p)) goto overflow;
 			/* can't check for underflow, as we have argv[argc] */
 
-			p = BODY(author_req);
 			ARG_COUNT_CHECK("Authorization-Request", pkt->author_req);
 			DECODE_FIELD_UINT8(attr_tacacs_packet_body_type, FR_PACKET_BODY_TYPE_REQUEST);
 
@@ -823,7 +821,6 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu
 			if (data_len > (size_t) (end - p)) goto overflow;
 			/* can't check for underflow, as we have argv[argc] */
 
-			p = BODY(author_reply);
 			ARG_COUNT_CHECK("Authorization-Reply", pkt->author_reply);
 			DECODE_FIELD_UINT8(attr_tacacs_packet_body_type, FR_PACKET_BODY_TYPE_RESPONSE);
 
@@ -886,7 +883,6 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu
 			if (data_len > (size_t) (end - p)) goto overflow;
 			/* can't check for underflow, as we have argv[argc] */
 
-			p = BODY(acct_req);
 			ARG_COUNT_CHECK("Accounting-Request", pkt->acct_req);
 			DECODE_FIELD_UINT8(attr_tacacs_packet_body_type, FR_PACKET_BODY_TYPE_REQUEST);