From: Alan T. DeKok <aland@freeradius.org>
Date: Tue, 14 Feb 2023 22:53:46 +0000 (-0500)
Subject: use "buffer" which is raw data, instead of "pkt" which is a struct
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1fbb4df6557fa3f42a0bd50fd36b70dfcaad0d1d;p=thirdparty%2Ffreeradius-server.git

use "buffer" which is raw data, instead of "pkt" which is a struct

If we take a pointer to the end of the struct, Coverity complains
that we're over-running the struct, even if there's lots of data
in the buffer where the struct is pointing
---

diff --git a/src/protocols/tacacs/decode.c b/src/protocols/tacacs/decode.c
index 8e27e00e843..32e7577958d 100644
--- a/src/protocols/tacacs/decode.c
+++ b/src/protocols/tacacs/decode.c
@@ -125,7 +125,7 @@ int fr_tacacs_packet_to_code(fr_tacacs_packet_t const *pkt)
 }
 
 #define PACKET_HEADER_CHECK(_msg, _hdr) do { \
-	p = (uint8_t const *) &(_hdr); \
+	p = buffer + FR_HEADER_LENGTH; \
 	if (sizeof(_hdr) > (size_t) (end - p)) { \
 		fr_strerror_printf("Header for %s is too small (%zu < %zu)", _msg, end - (uint8_t const *) pkt, p - (uint8_t const *) pkt); \
 		goto fail; \
@@ -144,7 +144,7 @@ int fr_tacacs_packet_to_code(fr_tacacs_packet_t const *pkt)
 		goto fail; \
 	} \
 	argv = body; \
-	attrs = ((uint8_t const *) &(_hdr)) + data_len; \
+	attrs = buffer + FR_HEADER_LENGTH + data_len; \
 	body += _hdr.arg_cnt; \
 	p = attrs; \
 	for (int i = 0; i < _hdr.arg_cnt; i++) { \
@@ -460,6 +460,8 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu
 
 		/*
 		 *	We need that to decrypt the body content.
+		 *
+		 *	@todo - use thread-local storage to avoid allocations?
 		 */
 		decrypted = talloc_memdup(ctx, buffer, buffer_len);
 		if (!decrypted) {
@@ -484,6 +486,8 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu
 			*code = fr_tacacs_packet_to_code((fr_tacacs_packet_t const *) decrypted);
 			if (*code < 0) goto fail;
 		}
+
+		buffer = decrypted;
 	}
 
 #ifndef NDEBUG