From 14aba7110fdedfa06db6fa73a1e262340119a92e Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Tue, 2 Feb 2021 11:40:21 +0100 Subject: [PATCH 1/6] net/ieee802154: Invalidate frame in case of no address in relevant modes All addressing mode but IEEE802154_ADDR_MODE_NONE should have a valid address. If not, the frame is invalid. Signed-off-by: Tomasz Bursztyka --- subsys/net/l2/ieee802154/ieee802154_frame.c | 33 ++++++++++++--------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/subsys/net/l2/ieee802154/ieee802154_frame.c b/subsys/net/l2/ieee802154/ieee802154_frame.c index 1d479c85d9a71..42532eac41e01 100644 --- a/subsys/net/l2/ieee802154/ieee802154_frame.c +++ b/subsys/net/l2/ieee802154/ieee802154_frame.c @@ -89,10 +89,10 @@ struct ieee802154_fcf_seq *ieee802154_validate_fc_seq(uint8_t *buf, uint8_t **p_ return fs; } -static inline struct ieee802154_address_field * -validate_addr(uint8_t *buf, uint8_t **p_buf, uint8_t *length, - enum ieee802154_addressing_mode mode, - bool pan_id_compression) +static inline bool validate_addr(uint8_t *buf, uint8_t **p_buf, uint8_t *length, + enum ieee802154_addressing_mode mode, + bool pan_id_compression, + struct ieee802154_address_field **addr) { uint8_t len = 0; @@ -102,7 +102,8 @@ validate_addr(uint8_t *buf, uint8_t **p_buf, uint8_t *length, buf, mode, pan_id_compression); if (mode == IEEE802154_ADDR_MODE_NONE) { - return NULL; + *addr = NULL; + return true; } if (!pan_id_compression) { @@ -117,13 +118,15 @@ validate_addr(uint8_t *buf, uint8_t **p_buf, uint8_t *length, } if (len > *length) { - return NULL; + return false; } *p_buf += len; *length -= len; - return (struct ieee802154_address_field *)buf; + *addr = (struct ieee802154_address_field *)buf; + + return true; } #ifdef CONFIG_NET_L2_IEEE802154_SECURITY @@ -439,13 +442,15 @@ bool ieee802154_validate_frame(uint8_t *buf, uint8_t length, return false; } - mpdu->mhr.dst_addr = validate_addr(p_buf, &p_buf, &length, - mpdu->mhr.fs->fc.dst_addr_mode, - false); - - mpdu->mhr.src_addr = validate_addr(p_buf, &p_buf, &length, - mpdu->mhr.fs->fc.src_addr_mode, - (mpdu->mhr.fs->fc.pan_id_comp)); + if (!validate_addr(p_buf, &p_buf, &length, + mpdu->mhr.fs->fc.dst_addr_mode, + false, &mpdu->mhr.dst_addr) || + !validate_addr(p_buf, &p_buf, &length, + mpdu->mhr.fs->fc.src_addr_mode, + (mpdu->mhr.fs->fc.pan_id_comp), + &mpdu->mhr.src_addr)) { + return false; + } #ifdef CONFIG_NET_L2_IEEE802154_SECURITY if (mpdu->mhr.fs->fc.security_enabled) { From d303dab975ccd23a1b2d9df8b4ad2bda42f8c18a Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Tue, 2 Feb 2021 12:02:14 +0100 Subject: [PATCH 2/6] net/ieee802154: Each fragment should be at least of its header's length Not validating this length could lead to integer underflow and memory corruption. Signed-off-by: Tomasz Bursztyka --- .../net/l2/ieee802154/ieee802154_fragment.c | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/subsys/net/l2/ieee802154/ieee802154_fragment.c b/subsys/net/l2/ieee802154/ieee802154_fragment.c index 790c159b56048..241669d0ab2d1 100644 --- a/subsys/net/l2/ieee802154/ieee802154_fragment.c +++ b/subsys/net/l2/ieee802154/ieee802154_fragment.c @@ -215,6 +215,11 @@ void ieee802154_fragment(struct ieee802154_fragment_ctx *ctx, ctx->offset = ctx->processed >> 3; } +static inline uint8_t get_datagram_type(uint8_t *ptr) +{ + return ptr[0] & NET_FRAG_DISPATCH_MASK; +} + static inline uint16_t get_datagram_size(uint8_t *ptr) { return ((ptr[0] & 0x1F) << 8) | ptr[1]; @@ -344,8 +349,7 @@ static inline struct frag_cache *get_reass_cache(uint16_t size, uint16_t tag) static inline void fragment_append(struct net_pkt *pkt, struct net_buf *frag) { - if ((frag->data[0] & NET_FRAG_DISPATCH_MASK) == - NET_6LO_DISPATCH_FRAG1) { + if (get_datagram_type(frag->data) == NET_6LO_DISPATCH_FRAG1) { /* Always make sure first fragment is inserted first * This will be useful for fragment_cached_pkt_len() */ @@ -367,8 +371,7 @@ static inline size_t fragment_cached_pkt_len(struct net_pkt *pkt) while (frag) { uint16_t hdr_len = NET_6LO_FRAGN_HDR_LEN; - if ((frag->data[0] & NET_FRAG_DISPATCH_MASK) == - NET_6LO_DISPATCH_FRAG1) { + if (get_datagram_type(frag->data) == NET_6LO_DISPATCH_FRAG1) { hdr_len = NET_6LO_FRAG1_HDR_LEN; } @@ -396,8 +399,7 @@ static inline size_t fragment_cached_pkt_len(struct net_pkt *pkt) static inline uint16_t fragment_offset(struct net_buf *frag) { - if ((frag->data[0] & NET_FRAG_DISPATCH_MASK) == - NET_6LO_DISPATCH_FRAG1) { + if (get_datagram_type(frag->data) == NET_6LO_DISPATCH_FRAG1) { return 0; } @@ -435,8 +437,7 @@ static inline void fragment_remove_headers(struct net_pkt *pkt) while (frag) { uint16_t hdr_len = NET_6LO_FRAGN_HDR_LEN; - if ((frag->data[0] & NET_FRAG_DISPATCH_MASK) == - NET_6LO_DISPATCH_FRAG1) { + if (get_datagram_type(frag->data) == NET_6LO_DISPATCH_FRAG1) { hdr_len = NET_6LO_FRAG1_HDR_LEN; } @@ -486,18 +487,28 @@ static inline enum net_verdict fragment_add_to_cache(struct net_pkt *pkt) struct net_buf *frag; uint16_t size; uint16_t tag; + uint8_t type; + + frag = pkt->buffer; + type = get_datagram_type(frag->data); + + if ((type == NET_6LO_DISPATCH_FRAG1 && + frag->len < NET_6LO_FRAG1_HDR_LEN) || + (type == NET_6LO_DISPATCH_FRAGN && + frag->len < NET_6LO_FRAGN_HDR_LEN)) { + return NET_DROP; + } /* Parse total size of packet */ - size = get_datagram_size(pkt->buffer->data); + size = get_datagram_size(frag->data); /* Parse the datagram tag */ - tag = get_datagram_tag(pkt->buffer->data + + tag = get_datagram_tag(frag->data + NET_6LO_FRAG_DATAGRAM_SIZE_LEN); /* If there are no fragments in the cache means this frag * is the first one. So cache Rx pkt otherwise not. */ - frag = pkt->buffer; pkt->buffer = NULL; cache = get_reass_cache(size, tag); @@ -556,8 +567,7 @@ enum net_verdict ieee802154_reassemble(struct net_pkt *pkt) return NET_DROP; } - if ((pkt->buffer->data[0] & NET_FRAG_DISPATCH_MASK) >= - NET_6LO_DISPATCH_FRAG1) { + if (get_datagram_type(pkt->buffer->data) >= NET_6LO_DISPATCH_FRAG1) { return fragment_add_to_cache(pkt); } else { NET_DBG("No frag dispatch (%02x)", pkt->buffer->data[0]); From f91b219d882c8d2632170c0fbb46b1b83eb11392 Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Tue, 2 Feb 2021 12:20:32 +0100 Subject: [PATCH 3/6] net/ieee802154: Avoid NULL pointer de-reference in packet reassembly In case the very first fragment holds all the data already. Signed-off-by: Tomasz Bursztyka --- subsys/net/l2/ieee802154/ieee802154_fragment.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/subsys/net/l2/ieee802154/ieee802154_fragment.c b/subsys/net/l2/ieee802154/ieee802154_fragment.c index 241669d0ab2d1..7f1cb27ee4690 100644 --- a/subsys/net/l2/ieee802154/ieee802154_fragment.c +++ b/subsys/net/l2/ieee802154/ieee802154_fragment.c @@ -526,9 +526,11 @@ static inline enum net_verdict fragment_add_to_cache(struct net_pkt *pkt) fragment_append(cache->pkt, frag); if (fragment_cached_pkt_len(cache->pkt) == cache->size) { - /* Assign buffer back to input packet. */ - pkt->buffer = cache->pkt->buffer; - cache->pkt->buffer = NULL; + if (!first_frag) { + /* Assign buffer back to input packet. */ + pkt->buffer = cache->pkt->buffer; + cache->pkt->buffer = NULL; + } fragment_reconstruct_packet(pkt); From 96818abe775ab6d3a666ca0d0d9c8f9cd39260e3 Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Wed, 3 Feb 2021 09:11:47 +0100 Subject: [PATCH 4/6] net/ieee802154: Make sure L2 drop any ACK frames Though ACK frames are not meant to reach L2 (drivers must ensure this never happens), let's "re-enforce" the L2 by dropping them. Signed-off-by: Tomasz Bursztyka --- subsys/net/l2/ieee802154/ieee802154.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/subsys/net/l2/ieee802154/ieee802154.c b/subsys/net/l2/ieee802154/ieee802154.c index da03e31b322fe..cb77997dabf1c 100644 --- a/subsys/net/l2/ieee802154/ieee802154.c +++ b/subsys/net/l2/ieee802154/ieee802154.c @@ -189,6 +189,10 @@ static enum net_verdict ieee802154_recv(struct net_if *iface, return NET_DROP; } + if (mpdu.mhr.fs->fc.frame_type == IEEE802154_FRAME_TYPE_ACK) { + return NET_DROP; + } + if (mpdu.mhr.fs->fc.frame_type == IEEE802154_FRAME_TYPE_BEACON) { return ieee802154_handle_beacon(iface, &mpdu, net_pkt_ieee802154_lqi(pkt)); From 68ee84f91a89f5bb011b656587644ad07401eee5 Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Wed, 3 Feb 2021 10:02:00 +0100 Subject: [PATCH 5/6] net/ieee802154: Do not unreference one time too many a fragmented packet In case the current packet is the same as the cached one, let's not unreference it while clearing the cache. Signed-off-by: Tomasz Bursztyka --- subsys/net/l2/ieee802154/ieee802154_fragment.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/subsys/net/l2/ieee802154/ieee802154_fragment.c b/subsys/net/l2/ieee802154/ieee802154_fragment.c index 7f1cb27ee4690..5a16f9b185d55 100644 --- a/subsys/net/l2/ieee802154/ieee802154_fragment.c +++ b/subsys/net/l2/ieee802154/ieee802154_fragment.c @@ -530,8 +530,14 @@ static inline enum net_verdict fragment_add_to_cache(struct net_pkt *pkt) /* Assign buffer back to input packet. */ pkt->buffer = cache->pkt->buffer; cache->pkt->buffer = NULL; + } else { + /* in case pkt == cache->pkt, we don't want + * to unref it while clearing the cach. + */ + cache->pkt = NULL; } + fragment_reconstruct_packet(pkt); /* Once reassemble is done, cache is no longer needed. */ From 5bf30efe09b8e48cf39fe97c2854ecfabfc487a3 Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Wed, 3 Feb 2021 10:07:54 +0100 Subject: [PATCH 6/6] net/ieee802154: Drop fragmented packet if first frag is not present Bogus fragmented packet could be sent without a FRAG1 fragment and hit reassembly. Let's make sure this does not happen. Signed-off-by: Tomasz Bursztyka --- subsys/net/l2/ieee802154/ieee802154_fragment.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/subsys/net/l2/ieee802154/ieee802154_fragment.c b/subsys/net/l2/ieee802154/ieee802154_fragment.c index 5a16f9b185d55..8d691bfede41d 100644 --- a/subsys/net/l2/ieee802154/ieee802154_fragment.c +++ b/subsys/net/l2/ieee802154/ieee802154_fragment.c @@ -472,6 +472,11 @@ static inline void fragment_reconstruct_packet(struct net_pkt *pkt) fragment_remove_headers(pkt); } +static inline bool fragment_packet_valid(struct net_pkt *pkt) +{ + return (get_datagram_type(pkt->buffer->data) == NET_6LO_DISPATCH_FRAG1); +} + /** * Parse size and tag from the fragment, check if we have any cache * related to it. If not create a new cache. @@ -537,11 +542,14 @@ static inline enum net_verdict fragment_add_to_cache(struct net_pkt *pkt) cache->pkt = NULL; } + clear_reass_cache(size, tag); - fragment_reconstruct_packet(pkt); + if (!fragment_packet_valid(pkt)) { + NET_ERR("Invalid fragmented packet"); + return NET_DROP; + } - /* Once reassemble is done, cache is no longer needed. */ - clear_reass_cache(size, tag); + fragment_reconstruct_packet(pkt); if (!net_6lo_uncompress(pkt)) { NET_ERR("Could not uncompress. Bogus packet?");