From af92de5eebf3da63f806f70415ad89b791cf45f4 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 5 Nov 2022 21:18:36 +0900 Subject: [PATCH 1/4] processudp: fix memory leak send/recvchalresponse should be freed before early returns in the function, as there are code paths where they would be allocated before these. Note free is no-op on null pointer so checking for non-null value is useless. Reported-by: Coverity#375342 --- l2tpns.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/l2tpns.c b/l2tpns.c index 2eb39bf..8bbe46c 100644 --- a/l2tpns.c +++ b/l2tpns.c @@ -2705,6 +2705,8 @@ void processudp(uint8_t *buf, int len, struct sockaddr_in *addr, uint16_t indexu { LOG(1, s, t, "Invalid length in AVP\n"); STAT(tunnel_rx_errors); + free(sendchalresponse); + free(recvchalresponse); return; } p += n; // next @@ -3250,6 +3252,8 @@ void processudp(uint8_t *buf, int len, struct sockaddr_in *addr, uint16_t indexu controladd(c, asession, t); // send the message } + free(sendchalresponse); + free(recvchalresponse); return; case 11: // ICRP LOG(3, s, t, "Received ICRP\n"); @@ -3304,8 +3308,8 @@ void processudp(uint8_t *buf, int len, struct sockaddr_in *addr, uint16_t indexu LOG(1, s, t, "Unknown message type %u\n", message); break; } - if (sendchalresponse) free(sendchalresponse); - if (recvchalresponse) free(recvchalresponse); + free(sendchalresponse); + free(recvchalresponse); cluster_send_tunnel(t); } else From bbedc40bf24f51e01c3fe9e35983744db9181ae2 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 5 Nov 2022 21:25:31 +0900 Subject: [PATCH 2/4] sendarp: fix out of bound read on mac address mac address is only 6 bytes, which we specify in sll_halen, so do not try to read more than that into sll_addr. Reported-by: Coverity#375313 --- arp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arp.c b/arp.c index 0aae069..2b4c664 100644 --- a/arp.c +++ b/arp.c @@ -55,7 +55,7 @@ void sendarp(int ifr_idx, const unsigned char* mac, in_addr_t ip) memset(&sll, 0, sizeof(sll)); sll.sll_family = AF_PACKET; - memcpy(sll.sll_addr, mac, sizeof(sll.sll_addr) - 1); + memcpy(sll.sll_addr, mac, ETH_ALEN); sll.sll_halen = ETH_ALEN; sll.sll_ifindex = ifr_idx; From 54be5008888bb26659416dec86a6b0cb3990aa9a Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 5 Nov 2022 21:29:00 +0900 Subject: [PATCH 3/4] bgp_write: fix sent data (offset) on partial write &peer->outbuf->packet has a non-1 size, so &foo + offset would incorrectly offset by sizeof(packet) * offset, while it is meant as a byte offset. Cast to char * to have a simple offset. Reported-by: Coverity#375309 --- bgp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgp.c b/bgp.c index 2791ca3..181de9a 100644 --- a/bgp.c +++ b/bgp.c @@ -1028,7 +1028,7 @@ static int bgp_write(struct bgp_peer *peer) int len = htons(peer->outbuf->packet.header.len); int r; - while ((r = write(peer->sock, &peer->outbuf->packet + peer->outbuf->done, + while ((r = write(peer->sock, (char*)&peer->outbuf->packet + peer->outbuf->done, len - peer->outbuf->done)) == -1) { if (errno == EINTR) From c770205890407bf0ebe978c517f5b669c0d6bcc6 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 5 Nov 2022 21:35:53 +0900 Subject: [PATCH 4/4] pppoe_sess_send: check packet length before reading header if the packet is too small then reading pack + ETH_HLEN is invalid, first check that the packet is big enough then read the header at an offset we know is valid Reported-by: Coverity#375305 --- pppoe.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pppoe.c b/pppoe.c index c6bc12d..28d0661 100644 --- a/pppoe.c +++ b/pppoe.c @@ -343,7 +343,7 @@ static void pppoe_disc_send(const uint8_t *pack) void pppoe_sess_send(const uint8_t *pack, uint16_t l, tunnelidt t) { - struct pppoe_hdr *hdr = (struct pppoe_hdr *)(pack + ETH_HLEN); + struct pppoe_hdr *hdr; int n; uint16_t sizeppp; sessionidt s; @@ -354,6 +354,13 @@ void pppoe_sess_send(const uint8_t *pack, uint16_t l, tunnelidt t) return; } + if (l < (ETH_HLEN + sizeof(*hdr) + 3)) + { + LOG(3, 0, t, "ERROR pppoe_sess_send: packet too small for pppoe sent (size=%d)\n", l); + return; + } + + hdr = (struct pppoe_hdr *)(pack + ETH_HLEN); s = ntohs(hdr->sid); if (session[s].tunnel != t) { @@ -361,12 +368,6 @@ void pppoe_sess_send(const uint8_t *pack, uint16_t l, tunnelidt t) return; } - if (l < (ETH_HLEN + sizeof(*hdr) + 3)) - { - LOG(0, s, t, "ERROR pppoe_sess_send: packet too small for pppoe sent (size=%d)\n", l); - return; - } - // recalculate the ppp frame length sizeppp = l - (ETH_HLEN + sizeof(*hdr)); hdr->length = htons(sizeppp);