If the length checks trigger, roa is leaked.  It makes more sense to me
to copy the data into ip4 and ip6, check lengths and then calloc rather
than the current order, so I moved the calloc down a bit. Alternatively,
we could just add a free(roa) before the return -1 in the length checks.

Index: rtr_proto.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rtr_proto.c,v
retrieving revision 1.5
diff -u -p -U4 -r1.5 rtr_proto.c
--- rtr_proto.c 6 Feb 2022 09:51:19 -0000       1.5
+++ rtr_proto.c 8 Mar 2022 12:26:29 -0000
@@ -441,23 +441,23 @@ rtr_parse_ipv4_prefix(struct rtr_session
                return -1;
        }
 
        memcpy(&ip4, buf + sizeof(struct rtr_header), sizeof(ip4));
-
-       if ((roa = calloc(1, sizeof(*roa))) == NULL) {
-               log_warn("rtr %s: received %s",
-                   log_rtr(rs), log_rtr_type(IPV4_PREFIX));
-               rtr_send_error(rs, INTERNAL_ERROR, "out of memory", NULL, 0);
-               return -1;
-       }
        if (ip4.prefixlen > 32 || ip4.maxlen > 32 ||
            ip4.prefixlen > ip4.maxlen) {
                log_warnx("rtr: %s: received %s: bad prefixlen / maxlen",
                    log_rtr(rs), log_rtr_type(IPV4_PREFIX));
                rtr_send_error(rs, CORRUPT_DATA, "bad prefixlen / maxlen",
                    buf, len);
                return -1;
        }
+
+       if ((roa = calloc(1, sizeof(*roa))) == NULL) {
+               log_warn("rtr %s: received %s",
+                   log_rtr(rs), log_rtr_type(IPV4_PREFIX));
+               rtr_send_error(rs, INTERNAL_ERROR, "out of memory", NULL, 0);
+               return -1;
+       }
        roa->aid = AID_INET;
        roa->prefixlen = ip4.prefixlen;
        roa->maxlen = ip4.maxlen;
        roa->asnum = ntohl(ip4.asnum);
@@ -510,21 +510,21 @@ rtr_parse_ipv6_prefix(struct rtr_session
                return -1;
        }
 
        memcpy(&ip6, buf + sizeof(struct rtr_header), sizeof(ip6));
-
-       if ((roa = calloc(1, sizeof(*roa))) == NULL) {
-               log_warn("rtr %s: received %s",
-                   log_rtr(rs), log_rtr_type(IPV6_PREFIX));
-               rtr_send_error(rs, INTERNAL_ERROR, "out of memory", NULL, 0);
-               return -1;
-       }
        if (ip6.prefixlen > 128 || ip6.maxlen > 128 ||
            ip6.prefixlen > ip6.maxlen) {
                log_warnx("rtr: %s: received %s: bad prefixlen / maxlen",
                    log_rtr(rs), log_rtr_type(IPV6_PREFIX));
                rtr_send_error(rs, CORRUPT_DATA, "bad prefixlen / maxlen",
                    buf, len);
+               return -1;
+       }
+
+       if ((roa = calloc(1, sizeof(*roa))) == NULL) {
+               log_warn("rtr %s: received %s",
+                   log_rtr(rs), log_rtr_type(IPV6_PREFIX));
+               rtr_send_error(rs, INTERNAL_ERROR, "out of memory", NULL, 0);
                return -1;
        }
        roa->aid = AID_INET6;
        roa->prefixlen = ip6.prefixlen;

Reply via email to