From d49810329dcf1ea162d383557136be4770818259 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Wed, 7 Dec 2022 11:48:47 -0300 Subject: [PATCH 1/2] pimd: fix MSDP crash on unexpected TLV sizes Increase the MSDP peer stream buffer size to handle the whole TLV (maximum is 65KiB due to 16bit field). If the stream is not resized there will be a crash in the read function attempting to put more than 9192 (`PIM_MSDP_SA_TLV_MAX_SIZE`) bytes. According to the RFC 3618 Section 12 we should accept the TLV and we should not reset the peer connection. Signed-off-by: Rafael Zalamena --- pimd/pim_msdp_packet.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/pimd/pim_msdp_packet.c b/pimd/pim_msdp_packet.c index 1152dd6f67..9f0cba03c2 100644 --- a/pimd/pim_msdp_packet.c +++ b/pimd/pim_msdp_packet.c @@ -711,6 +711,30 @@ void pim_msdp_read(struct thread *thread) pim_msdp_pkt_rxed_with_fatal_error(mp); return; } + + /* + * Handle messages with longer than expected TLV size: resize + * the stream to handle reading the whole message. + * + * RFC 3618 Section 12. 'Packet Formats': + * > ... If an implementation receives a TLV whose length + * > exceeds the maximum TLV length specified below, the TLV + * > SHOULD be accepted. Any additional data, including possible + * > next TLV's in the same message, SHOULD be ignored, and the + * > MSDP session should not be reset. ... + */ + if (len > PIM_MSDP_SA_TLV_MAX_SIZE) { + /* Check if the current buffer is big enough. */ + if (mp->ibuf->size < len) { + if (PIM_DEBUG_MSDP_PACKETS) + zlog_debug( + "MSDP peer %s sent TLV with unexpected large length (%d bytes)", + mp->key_str, len); + + stream_resize_inplace(&mp->ibuf, len); + } + } + /* read complete TLV */ mp->packet_size = len; } From 1dd422a22bbb5893dd0872bf5aeb6ec4dab6f98f Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Wed, 7 Dec 2022 11:49:26 -0300 Subject: [PATCH 2/2] pimd: fix MSDP packet debug crashes Add some safe guards to avoid crashes and alert us about programming errors in packet build. Signed-off-by: Rafael Zalamena --- pimd/pim_msdp_packet.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pimd/pim_msdp_packet.c b/pimd/pim_msdp_packet.c index 9f0cba03c2..5230f6a332 100644 --- a/pimd/pim_msdp_packet.c +++ b/pimd/pim_msdp_packet.c @@ -83,10 +83,18 @@ static void pim_msdp_pkt_sa_dump_one(struct stream *s) static void pim_msdp_pkt_sa_dump(struct stream *s) { + const size_t header_length = PIM_MSDP_SA_X_SIZE - PIM_MSDP_HEADER_SIZE; + size_t payload_length; int entry_cnt; int i; struct in_addr rp; /* Last RP address associated with this SA */ + if (header_length > STREAM_READABLE(s)) { + zlog_err("BUG MSDP SA bad header (readable %zu expected %zu)", + STREAM_READABLE(s), header_length); + return; + } + entry_cnt = stream_getc(s); rp.s_addr = stream_get_ipv4(s); @@ -96,6 +104,13 @@ static void pim_msdp_pkt_sa_dump(struct stream *s) zlog_debug(" entry_cnt %d rp %s", entry_cnt, rp_str); } + payload_length = (size_t)entry_cnt * PIM_MSDP_SA_ONE_ENTRY_SIZE; + if (payload_length > STREAM_READABLE(s)) { + zlog_err("BUG MSDP SA bad length (readable %zu expected %zu)", + STREAM_READABLE(s), payload_length); + return; + } + /* dump SAs */ for (i = 0; i < entry_cnt; ++i) { pim_msdp_pkt_sa_dump_one(s); @@ -116,6 +131,11 @@ static void pim_msdp_pkt_dump(struct pim_msdp_peer *mp, int type, int len, return; } + if (len < PIM_MSDP_HEADER_SIZE) { + zlog_err("invalid MSDP header length"); + return; + } + switch (type) { case PIM_MSDP_V4_SOURCE_ACTIVE: pim_msdp_pkt_sa_dump(s);