Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: avoid empty allocations
To:
tech@openbsd.org
Date:
Tue, 5 Nov 2024 18:49:08 +0100

Download raw body.

Thread
The allocation of a zero-sized object is implementation-defined. In
particular, NULL could be returned and we would error out for the wrong
reason.

Not all these are reachable since the parsers enforce that the sizes are
positive for ASPA and ROA, but I think we're better off not to rely
on that. Both the ones in cert.c are reachable as is the one in spl.c.

Index: aspa.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/aspa.c,v
diff -u -p -r1.30 aspa.c
--- aspa.c	8 Apr 2024 14:02:13 -0000	1.30
+++ aspa.c	5 Nov 2024 17:36:26 -0000
@@ -290,9 +290,14 @@ aspa_read(struct ibuf *b)
 	io_read_buf(b, &p->expires, sizeof(p->expires));
 
 	io_read_buf(b, &p->providersz, sizeof(size_t));
-	if ((p->providers = calloc(p->providersz, sizeof(uint32_t))) == NULL)
-		err(1, NULL);
-	io_read_buf(b, p->providers, p->providersz * sizeof(p->providers[0]));
+
+	if (p->providersz > 0) {
+		if ((p->providers = calloc(p->providersz,
+		    sizeof(p->providers[0]))) == NULL)
+			err(1, NULL);
+		io_read_buf(b, p->providers,
+		    p->providersz * sizeof(p->providers[0]));
+	}
 
 	io_read_str(b, &p->aia);
 	io_read_str(b, &p->aki);
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.151 cert.c
--- cert.c	7 Oct 2024 12:19:52 -0000	1.151
+++ cert.c	5 Nov 2024 17:36:26 -0000
@@ -1208,15 +1208,19 @@ cert_read(struct ibuf *b)
 	io_read_buf(b, &p->ipsz, sizeof(p->ipsz));
 	io_read_buf(b, &p->asz, sizeof(p->asz));
 
-	p->ips = calloc(p->ipsz, sizeof(struct cert_ip));
-	if (p->ips == NULL)
-		err(1, NULL);
-	io_read_buf(b, p->ips, p->ipsz * sizeof(p->ips[0]));
+	if (p->ipsz > 0) {
+		p->ips = calloc(p->ipsz, sizeof(struct cert_ip));
+		if (p->ips == NULL)
+			err(1, NULL);
+		io_read_buf(b, p->ips, p->ipsz * sizeof(p->ips[0]));
+	}
 
-	p->as = calloc(p->asz, sizeof(struct cert_as));
-	if (p->as == NULL)
-		err(1, NULL);
-	io_read_buf(b, p->as, p->asz * sizeof(p->as[0]));
+	if (p->asz > 0) {
+		p->as = calloc(p->asz, sizeof(struct cert_as));
+		if (p->as == NULL)
+			err(1, NULL);
+		io_read_buf(b, p->as, p->asz * sizeof(p->as[0]));
+	}
 
 	io_read_str(b, &p->mft);
 	io_read_str(b, &p->notify);
Index: roa.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/roa.c,v
diff -u -p -r1.78 roa.c
--- roa.c	24 May 2024 12:57:20 -0000	1.78
+++ roa.c	5 Nov 2024 17:36:26 -0000
@@ -289,6 +289,11 @@ roa_parse(X509 **x509, const char *fn, i
 		goto out;
 	}
 
+	if (cert->ipsz == 0) {
+		warnx("%s: no IP address present", fn);
+		goto out;
+	}
+
 	/*
 	 * If the ROA isn't valid, we accept it anyway and depend upon
 	 * the code around roa_read() to check the "valid" field itself.
@@ -365,9 +370,11 @@ roa_read(struct ibuf *b)
 	io_read_buf(b, &p->ipsz, sizeof(p->ipsz));
 	io_read_buf(b, &p->expires, sizeof(p->expires));
 
-	if ((p->ips = calloc(p->ipsz, sizeof(struct roa_ip))) == NULL)
-		err(1, NULL);
-	io_read_buf(b, p->ips, p->ipsz * sizeof(p->ips[0]));
+	if (p->ipsz > 0) {
+		if ((p->ips = calloc(p->ipsz, sizeof(p->ips[0]))) == NULL)
+			err(1, NULL);
+		io_read_buf(b, p->ips, p->ipsz * sizeof(p->ips[0]));
+	}
 
 	io_read_str(b, &p->aia);
 	io_read_str(b, &p->aki);
Index: spl.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/spl.c,v
diff -u -p -r1.3 spl.c
--- spl.c	15 May 2024 14:43:32 -0000	1.3
+++ spl.c	5 Nov 2024 17:36:26 -0000
@@ -373,9 +373,11 @@ spl_read(struct ibuf *b)
 	io_read_buf(b, &s->pfxsz, sizeof(s->pfxsz));
 	io_read_buf(b, &s->expires, sizeof(s->expires));
 
-	if ((s->pfxs = calloc(s->pfxsz, sizeof(struct spl_pfx))) == NULL)
-		err(1, NULL);
-	io_read_buf(b, s->pfxs, s->pfxsz * sizeof(s->pfxs[0]));
+	if (s->pfxs > 0) {
+		if ((s->pfxs = calloc(s->pfxsz, sizeof(s->pfxs[0]))) == NULL)
+			err(1, NULL);
+		io_read_buf(b, s->pfxs, s->pfxsz * sizeof(s->pfxs[0]));
+	}
 
 	io_read_str(b, &s->aia);
 	io_read_str(b, &s->aki);