From: Job Snijders <job@openbsd.org>
Subject: rpki-client: check whether all data in eContent has been consumed
To: tech@openbsd.org
Date: Mon, 5 Feb 2024 19:08:48 +0000

It is possible that a given ASN.1 template generated d2i_*() function
didn't consume all data, so there is a potential for malleability?

The solution is to have the callers check whether everything was
consumed: if not, error out.

We already do this for CMS/CRL/X509, this diff extends the pattern to
eContent handling.

OK?

Kind regards,

Job

Index: aspa.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/aspa.c,v
diff -u -p -r1.24 aspa.c
--- aspa.c	13 Oct 2023 12:06:49 -0000	1.24
+++ aspa.c	5 Feb 2024 18:59:10 -0000
@@ -129,11 +129,18 @@ aspa_parse_providers(struct parse *p, co
 static int
 aspa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 {
+	const unsigned char	*oder;
 	ASProviderAttestation	*aspa;
 	int			 rc = 0;
 
+	oder = d;
 	if ((aspa = d2i_ASProviderAttestation(NULL, &d, dsz)) == NULL) {
 		warnx("%s: ASPA: failed to parse ASProviderAttestation", p->fn);
+		goto out;
+	}
+	if (d != oder + dsz) {
+		warnx("%s: %td bytes trailing garbage in eContent", p->fn,
+		    oder + dsz - d);
 		goto out;
 	}
 
Index: mft.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
diff -u -p -r1.105 mft.c
--- mft.c	4 Feb 2024 00:53:27 -0000	1.105
+++ mft.c	5 Feb 2024 18:59:11 -0000
@@ -232,13 +232,20 @@ mft_parse_filehash(struct parse *p, cons
 static int
 mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 {
+	const unsigned char	*oder;
 	Manifest		*mft;
 	FileAndHash		*fh;
 	int			 i, rc = 0;
 
+	oder = d;
 	if ((mft = d2i_Manifest(NULL, &d, dsz)) == NULL) {
 		warnx("%s: RFC 6486 section 4: failed to parse Manifest",
 		    p->fn);
+		goto out;
+	}
+	if (d != oder + dsz) {
+		warnx("%s: %td bytes trailing garbage in eContent", p->fn,
+		    oder + dsz - d);
 		goto out;
 	}
 
Index: roa.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/roa.c,v
diff -u -p -r1.72 roa.c
--- roa.c	14 Dec 2023 07:52:53 -0000	1.72
+++ roa.c	5 Feb 2024 18:59:11 -0000
@@ -101,6 +101,7 @@ IMPLEMENT_ASN1_FUNCTIONS(RouteOriginAtte
 static int
 roa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 {
+	const unsigned char		*oder;
 	RouteOriginAttestation		*roa;
 	const ROAIPAddressFamily	*addrfam;
 	const STACK_OF(ROAIPAddress)	*addrs;
@@ -113,9 +114,15 @@ roa_parse_econtent(const unsigned char *
 	int				 ipaddrblocksz;
 	int				 i, j, rc = 0;
 
+	oder = d;
 	if ((roa = d2i_RouteOriginAttestation(NULL, &d, dsz)) == NULL) {
 		warnx("%s: RFC 6482 section 3: failed to parse "
 		    "RouteOriginAttestation", p->fn);
+		goto out;
+	}
+	if (d != oder + dsz) {
+		warnx("%s: %td bytes trailing garbage in eContent", p->fn,
+		    oder + dsz - d);
 		goto out;
 	}
 
Index: rsc.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/rsc.c,v
diff -u -p -r1.29 rsc.c
--- rsc.c	13 Oct 2023 12:06:49 -0000	1.29
+++ rsc.c	5 Feb 2024 18:59:11 -0000
@@ -325,6 +325,7 @@ rsc_parse_checklist(struct parse *p, con
 static int
 rsc_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 {
+	const unsigned char	*oder;
 	RpkiSignedChecklist	*rsc;
 	ResourceBlock		*resources;
 	int			 rc = 0;
@@ -333,8 +334,14 @@ rsc_parse_econtent(const unsigned char *
 	 * RFC 9323 section 4
 	 */
 
+	oder = d;
 	if ((rsc = d2i_RpkiSignedChecklist(NULL, &d, dsz)) == NULL) {
 		warnx("%s: RSC: failed to parse RpkiSignedChecklist", p->fn);
+		goto out;
+	}
+	if (d != oder + dsz) {
+		warnx("%s: %td bytes trailing garbage in eContent", p->fn,
+		    oder + dsz - d);
 		goto out;
 	}
 
Index: tak.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/tak.c,v
diff -u -p -r1.13 tak.c
--- tak.c	13 Oct 2023 12:06:49 -0000	1.13
+++ tak.c	5 Feb 2024 18:59:11 -0000
@@ -184,14 +184,21 @@ parse_takey(const char *fn, const TAKey 
 static int
 tak_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 {
-	TAK		*tak;
-	const char	*fn;
-	int		 rc = 0;
+	const unsigned char	*oder;
+	TAK			*tak;
+	const char		*fn;
+	int			 rc = 0;
 
 	fn = p->fn;
 
+	oder = d;
 	if ((tak = d2i_TAK(NULL, &d, dsz)) == NULL) {
 		warnx("%s: failed to parse Trust Anchor Key", fn);
+		goto out;
+	}
+	if (d != oder + dsz) {
+		warnx("%s: %td bytes trailing garbage in eContent", p->fn,
+		    oder + dsz - d);
 		goto out;
 	}