From: claudio <claudio@openbsd.org>
Date: Tue, 10 Sep 2024 09:38:45 +0000 (+0000)
Subject: Be more careful with aspath that have 0 length (aka the empty AS_PATH).
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=e386eeb169001ba40fb5933f7e7f4e432772e268;p=openbsd

Be more careful with aspath that have 0 length (aka the empty AS_PATH).

Again malloc(0) is not portable and calling memcpy with a NULL pointer
and a 0 length is not allowed by the C standard.

OK tb@
---

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index fb58c5ec54d..4cd26fa6365 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.632 2024/09/09 15:00:45 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.633 2024/09/10 09:38:45 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -1123,7 +1123,9 @@ rde_dispatch_imsg_parent(struct imsgbuf *imsgbuf)
 			    sizeof(uint32_t));
 			break;
 		case IMSG_RECONF_AS_SET_ITEMS:
-			if (imsg_get_ibuf(&imsg, &ibuf) == -1)
+			if (imsg_get_ibuf(&imsg, &ibuf) == -1 ||
+			    ibuf_size(&ibuf) == 0 ||
+			    ibuf_size(&ibuf) % sizeof(uint32_t) != 0)
 				fatalx("IMSG_RECONF_AS_SET_ITEMS bad len");
 			nmemb = ibuf_size(&ibuf) / sizeof(uint32_t);
 			if (set_add(last_as_set->set, ibuf_data(&ibuf),
diff --git a/usr.sbin/bgpd/rde_attr.c b/usr.sbin/bgpd/rde_attr.c
index b1b4957475b..2d67efa829a 100644
--- a/usr.sbin/bgpd/rde_attr.c
+++ b/usr.sbin/bgpd/rde_attr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_attr.c,v 1.134 2023/07/12 14:45:43 claudio Exp $ */
+/*	$OpenBSD: rde_attr.c,v 1.135 2024/09/10 09:38:45 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -357,7 +357,8 @@ aspath_get(void *data, uint16_t len)
 	aspath->len = len;
 	aspath->ascnt = aspath_count(data, len);
 	aspath->source_as = aspath_extract_origin(data, len);
-	memcpy(aspath->data, data, len);
+	if (len != 0)
+		memcpy(aspath->data, data, len);
 
 	return (aspath);
 }
@@ -396,7 +397,7 @@ aspath_put(struct aspath *aspath)
 u_char *
 aspath_deflate(u_char *data, uint16_t *len, int *flagnew)
 {
-	uint8_t	*seg, *nseg, *ndata;
+	uint8_t		*seg, *nseg, *ndata = NULL;
 	uint32_t	 as;
 	int		 i;
 	uint16_t	 seg_size, olen, nlen;
@@ -415,6 +416,9 @@ aspath_deflate(u_char *data, uint16_t *len, int *flagnew)
 			fatalx("%s: would overflow", __func__);
 	}
 
+	if (nlen == 0)
+		goto done;
+
 	if ((ndata = malloc(nlen)) == NULL)
 		fatal("%s", __func__);
 
@@ -437,6 +441,7 @@ aspath_deflate(u_char *data, uint16_t *len, int *flagnew)
 		}
 	}
 
+ done:
 	*len = nlen;
 	return (ndata);
 }
@@ -791,6 +796,10 @@ aspath_prepend(struct aspath *asp, uint32_t as, int quantum, uint16_t *len)
 		fatalx("aspath_prepend: preposterous prepend");
 	if (quantum == 0) {
 		/* no change needed but return a copy */
+		if (asp->len == 0) {
+			*len = 0;
+			return (NULL);
+		}
 		p = malloc(asp->len);
 		if (p == NULL)
 			fatal("%s", __func__);
@@ -834,7 +843,8 @@ aspath_prepend(struct aspath *asp, uint32_t as, int quantum, uint16_t *len)
 			wpos += sizeof(uint32_t);
 		}
 	}
-	memcpy(p + wpos, asp->data + shift, asp->len - shift);
+	if (asp->len > shift)
+		memcpy(p + wpos, asp->data + shift, asp->len - shift);
 
 	*len = l;
 	return (p);
@@ -852,6 +862,11 @@ aspath_override(struct aspath *asp, uint32_t neighbor_as, uint32_t local_as,
 	uint16_t	 l, seg_size;
 	uint8_t		 i, seg_len, seg_type;
 
+	if (asp->len == 0) {
+		*len = 0;
+		return (NULL);
+	}
+
 	p = malloc(asp->len);
 	if (p == NULL)
 		fatal("%s", __func__);
diff --git a/usr.sbin/bgpd/rde_sets.c b/usr.sbin/bgpd/rde_sets.c
index 766f38d3ade..bcfd858b21a 100644
--- a/usr.sbin/bgpd/rde_sets.c
+++ b/usr.sbin/bgpd/rde_sets.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_sets.c,v 1.12 2022/07/28 13:11:51 deraadt Exp $ */
+/*	$OpenBSD: rde_sets.c,v 1.13 2024/09/10 09:38:45 claudio Exp $ */
 
 /*
  * Copyright (c) 2018 Claudio Jeker <claudio@openbsd.org>
@@ -149,6 +149,9 @@ set_free(struct set_table *set)
 int
 set_add(struct set_table *set, void *elms, size_t nelms)
 {
+	if (nelms == 0)		/* nothing todo */
+		return 0;
+
 	if (set->max < nelms || set->max - nelms < set->nmemb) {
 		uint32_t *s;
 		size_t new_size;