Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: rpki-client: leaks of uri in rrdp start_publish_*
To:
tech@openbsd.org
Date:
Fri, 13 Jun 2025 08:43:07 +0200

Download raw body.

Thread
> > There is a related leak in start_{delta,snapshot}_elem() that coverity
> > didn't spot: if the xml contains multiple "session_id", we leak the
> > ->session_id, but I believe we should just reject all repeated
> > attributes there. That's for a separate diff.

This way we avoid the leak and hit this error on any repeated attributes,
which I think is still correct:

		PARSE_FAIL(p,
                    "parse failed - non conforming "
                    "attribute '%s' found in snapshot elem", attr[i]);

Index: rrdp_delta.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/rrdp_delta.c,v
diff -u -p -r1.15 rrdp_delta.c
--- rrdp_delta.c	13 Jun 2025 06:20:27 -0000	1.15
+++ rrdp_delta.c	13 Jun 2025 06:41:01 -0000
@@ -61,22 +61,20 @@ start_delta_elem(struct delta_xml *dxml,
 	for (i = 0; attr[i]; i += 2) {
 		const char *errstr;
 		if (strcmp("xmlns", attr[i]) == 0 &&
-		    strcmp(RRDP_XMLNS, attr[i + 1]) == 0) {
-			has_xmlns = 1;
+		    strcmp(RRDP_XMLNS, attr[i + 1]) == 0 && has_xmlns++ == 0)
 			continue;
-		}
-		if (strcmp("version", attr[i]) == 0) {
+		if (strcmp("version", attr[i]) == 0 && dxml->version == 0) {
 			dxml->version = strtonum(attr[i + 1],
 			    1, MAX_VERSION, &errstr);
 			if (errstr == NULL)
 				continue;
 		}
 		if (strcmp("session_id", attr[i]) == 0 &&
-		    valid_uuid(attr[i + 1])) {
+		    valid_uuid(attr[i + 1]) && dxml->session_id == NULL) {
 			dxml->session_id = xstrdup(attr[i + 1]);
 			continue;
 		}
-		if (strcmp("serial", attr[i]) == 0) {
+		if (strcmp("serial", attr[i]) == 0 && dxml->serial == 0) {
 			dxml->serial = strtonum(attr[i + 1],
 			    1, LLONG_MAX, &errstr);
 			if (errstr == NULL)
Index: rrdp_snapshot.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/rrdp_snapshot.c,v
diff -u -p -r1.11 rrdp_snapshot.c
--- rrdp_snapshot.c	13 Jun 2025 06:20:27 -0000	1.11
+++ rrdp_snapshot.c	13 Jun 2025 06:41:01 -0000
@@ -59,22 +59,20 @@ start_snapshot_elem(struct snapshot_xml 
 	for (i = 0; attr[i]; i += 2) {
 		const char *errstr;
 		if (strcmp("xmlns", attr[i]) == 0 &&
-		    strcmp(RRDP_XMLNS, attr[i + 1]) == 0) {
-			has_xmlns = 1;
+		    strcmp(RRDP_XMLNS, attr[i + 1]) == 0 && has_xmlns++ == 0)
 			continue;
-		}
-		if (strcmp("version", attr[i]) == 0) {
+		if (strcmp("version", attr[i]) == 0 && sxml->version == 0) {
 			sxml->version = strtonum(attr[i + 1],
 			    1, MAX_VERSION, &errstr);
 			if (errstr == NULL)
 				continue;
 		}
 		if (strcmp("session_id", attr[i]) == 0 &&
-		    valid_uuid(attr[i + 1])) {
+		    valid_uuid(attr[i + 1]) && sxml->session_id == NULL) {
 			sxml->session_id = xstrdup(attr[i + 1]);
 			continue;
 		}
-		if (strcmp("serial", attr[i]) == 0) {
+		if (strcmp("serial", attr[i]) == 0 && sxml->serial == 0) {
 			sxml->serial = strtonum(attr[i + 1],
 			    1, LLONG_MAX, &errstr);
 			if (errstr == NULL)