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