On Thu, Feb 10, 2022 at 08:44:08AM +0100, Theo Buehler wrote:
> On Thu, Feb 10, 2022 at 07:51:45AM +0100, Theo Buehler wrote:
> > At this point conn->last_modified may or may not be allocated.
> > If it is, overriting it will leak 30 bytes.
>
> rrdp_input_handler() has a leak of the same kind.
>
> Index: http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 http.c
> --- http.c 23 Jan 2022 12:09:24 -0000 1.52
> +++ http.c 10 Feb 2022 02:09:38 -0000
> @@ -1231,6 +1231,7 @@ http_parse_header(struct http_connection
> } else if (strncasecmp(cp, LAST_MODIFIED,
> sizeof(LAST_MODIFIED) - 1) == 0) {
> cp += sizeof(LAST_MODIFIED) - 1;
> + free(conn->last_modified);
> if ((conn->last_modified = strdup(cp)) == NULL)
> err(1, NULL);
> }
This one is OK claudio@
> Index: rrdp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rrdp.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 rrdp.c
> --- rrdp.c 23 Jan 2022 12:09:24 -0000 1.21
> +++ rrdp.c 10 Feb 2022 07:41:54 -0000
> @@ -429,6 +429,7 @@ rrdp_input_handler(int fd)
> errx(1, "%s: bad internal state", s->local);
>
> s->res = res;
> + free(s->last_mod);
> s->last_mod = last_mod;
> s->state |= RRDP_STATE_HTTP_DONE;
> rrdp_finished(s);
>
This one needs more thought. rrdp_finished() -> notification_done() moves
the last_mod into nxml->current->last_mod and so you can end up with
either a double free or use after free. That code needs a proper refactor.
I think it would be best to remove last_mod from the rrdp state and just
pass it as char * to rrdp_finished. Still need to look when and how to
free the value reliably.
--
:wq Claudio
