From: martijn <martijn@openbsd.org>
Date: Tue, 20 Feb 2024 12:25:43 +0000 (+0000)
Subject: Remove the packetid logic from ax.c. It's not used by snmpd and has some
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=b26e50747e055dfe05cb79d9a57e1528b7acf1e5;p=openbsd

Remove the packetid logic from ax.c. It's not used by snmpd and has some
serious flaws.

Issue reported and analysis by Steffen Christgau.
OK tb@
---

diff --git a/usr.sbin/snmpd/ax.c b/usr.sbin/snmpd/ax.c
index 4a2509a2f78..f9bf6951847 100644
--- a/usr.sbin/snmpd/ax.c
+++ b/usr.sbin/snmpd/ax.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ax.c,v 1.4 2023/12/21 12:43:31 martijn Exp $ */
+/*	$OpenBSD: ax.c,v 1.5 2024/02/20 12:25:43 martijn Exp $ */
 /*
  * Copyright (c) 2019 Martijn van Duren <martijn@openbsd.org>
  *
@@ -36,7 +36,6 @@ static int ax_pdu_need(struct ax *, size_t);
 static int ax_pdu_header(struct ax *,
     enum ax_pdu_type, uint8_t, uint32_t, uint32_t, uint32_t,
     struct ax_ostring *);
-static uint32_t ax_packetid(struct ax *);
 static uint32_t ax_pdu_queue(struct ax *);
 static int ax_pdu_add_uint16(struct ax *, uint16_t);
 static int ax_pdu_add_uint32(struct ax *, uint32_t);
@@ -89,7 +88,6 @@ ax_free(struct ax *ax)
 	close(ax->ax_fd);
 	free(ax->ax_rbuf);
 	free(ax->ax_wbuf);
-	free(ax->ax_packetids);
 	free(ax);
 }
 
@@ -394,24 +392,6 @@ ax_recv(struct ax *ax)
 		}
 		break;
 	case AX_PDU_TYPE_RESPONSE:
-		if (ax->ax_packetids != NULL) {
-			found = 0;
-			for (i = 0; ax->ax_packetids[i] != 0; i++) {
-				if (ax->ax_packetids[i] ==
-				    pdu->ap_header.aph_packetid) {
-					packetidx = i;
-					found = 1;
-				}
-			}
-			if (found) {
-				ax->ax_packetids[packetidx] =
-				    ax->ax_packetids[i - 1];
-				ax->ax_packetids[i - 1] = 0;
-			} else {
-				errno = EPROTO;
-				goto fail;
-			}
-		}
 		if (rawlen < 8) {
 			errno = EPROTO;
 			goto fail;
@@ -543,7 +523,7 @@ uint32_t
 ax_close(struct ax *ax, uint32_t sessionid,
     enum ax_close_reason reason)
 {
-	if (ax_pdu_header(ax, AX_PDU_TYPE_CLOSE, 0, sessionid, 0, 0,
+	if (ax_pdu_header(ax, AX_PDU_TYPE_CLOSE, 0, sessionid, arc4random(), 0,
 	    NULL) == -1)
 		return 0;
 
@@ -1163,8 +1143,6 @@ ax_pdu_header(struct ax *ax, enum ax_pdu_type type, uint8_t flags,
 		flags |= AX_PDU_FLAG_NETWORK_BYTE_ORDER;
 	ax->ax_wbuf[ax->ax_wbtlen++] = flags;
 	ax->ax_wbuf[ax->ax_wbtlen++] = 0;
-	if (packetid == 0)
-		packetid = ax_packetid(ax);
 	if (ax_pdu_add_uint32(ax, sessionid) == -1 ||
 	    ax_pdu_add_uint32(ax, transactionid) == -1 ||
 	    ax_pdu_add_uint32(ax, packetid) == -1 ||
@@ -1179,40 +1157,6 @@ ax_pdu_header(struct ax *ax, enum ax_pdu_type type, uint8_t flags,
 	return 0;
 }
 
-static uint32_t
-ax_packetid(struct ax *ax)
-{
-	uint32_t packetid, *packetids;
-	size_t npackets = 0, i;
-	int found;
-
-	if (ax->ax_packetids != NULL) {
-		for (npackets = 0; ax->ax_packetids[npackets] != 0; npackets++)
-			continue;
-	}
-	if (ax->ax_packetidsize == 0 || npackets == ax->ax_packetidsize - 1) {
-		packetids = recallocarray(ax->ax_packetids, ax->ax_packetidsize,
-		    ax->ax_packetidsize + 25, sizeof(*packetids));
-		if (packetids == NULL)
-			return 0;
-		ax->ax_packetidsize += 25;
-		ax->ax_packetids = packetids;
-	}
-	do {
-		found = 0;
-		packetid = arc4random();
-		for (i = 0; ax->ax_packetids[i] != 0; i++) {
-			if (ax->ax_packetids[i] == packetid) {
-				found = 1;
-				break;
-			}
-		}
-	} while (packetid == 0 || found);
-	ax->ax_packetids[npackets] = packetid;
-
-	return packetid;
-}
-
 static int
 ax_pdu_add_uint16(struct ax *ax, uint16_t value)
 {
diff --git a/usr.sbin/snmpd/ax.h b/usr.sbin/snmpd/ax.h
index 36006221ab0..abfb70f12c9 100644
--- a/usr.sbin/snmpd/ax.h
+++ b/usr.sbin/snmpd/ax.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ax.h,v 1.3 2023/12/21 12:43:31 martijn Exp $ */
+/*	$OpenBSD: ax.h,v 1.4 2024/02/20 12:25:43 martijn Exp $ */
 /*
  * Copyright (c) 2019 Martijn van Duren <martijn@openbsd.org>
  *
@@ -125,8 +125,6 @@ struct ax {
 	size_t ax_wblen;
 	size_t ax_wbtlen;
 	size_t ax_wbsize;
-	uint32_t *ax_packetids;
-	size_t ax_packetidsize;
 };
 
 #ifndef AX_PRIMITIVE