Merge RIOT-OS#19497

19497: security fixes [backport 2023.04] r=kaspar030 a=MrKevinWeiss ### Contribution description ### Testing procedure ### Issues/PRs references Co-authored-by: Kaspar Schleiser <kaspar@schleiser.de>
maribu · Apr 24, 2023 · f46d71e · f46d71e
2 parents 0339069 + fce1e38
commit f46d71e
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 7 deletions.
diff --git a/sys/net/gnrc/network_layer/sixlowpan/frag/sfr/gnrc_sixlowpan_frag_sfr.c b/sys/net/gnrc/network_layer/sixlowpan/frag/sfr/gnrc_sixlowpan_frag_sfr.c
@@ -309,6 +309,10 @@ static int _forward_rfrag(gnrc_pktsnip_t *pkt, _generic_rb_entry_t *entry,
 /* ====== PUBLIC FUNCTION DEFINITIONS ====== */
 void gnrc_sixlowpan_frag_sfr_init(void)
 {
+    /* initialize _arq_timer if not yet done */
+    if (_arq_timer.callback == NULL) {
+        evtimer_init_msg(&_arq_timer);
+    }
     if (gnrc_sixlowpan_frag_sfr_congure_snd_has_inter_frame_gap()) {
         for (unsigned i = 0; i < FRAME_QUEUE_POOL_SIZE; i++) {
             clist_rpush(&_frame_queue_free, &_frame_queue_pool[i].super);
@@ -397,10 +401,6 @@ void gnrc_sixlowpan_frag_sfr_send(gnrc_pktsnip_t *pkt, void *ctx,
           _frag_seq(frag_desc), _frag_size(frag_desc),
           frag_desc->offset);
     if (_frag_ack_req(frag_desc)) {
-        /* initialize _arq_timer if not yet done */
-        if (_arq_timer.callback == NULL) {
-            evtimer_init_msg(&_arq_timer);
-        }
         _sched_arq_timeout(fbuf, fbuf->sfr.arq_timeout);
     }
 

diff --git a/sys/net/gnrc/network_layer/sixlowpan/iphc/gnrc_sixlowpan_iphc.c b/sys/net/gnrc/network_layer/sixlowpan/iphc/gnrc_sixlowpan_iphc.c
@@ -734,6 +734,14 @@ void gnrc_sixlowpan_iphc_recv(gnrc_pktsnip_t *sixlo, void *rbuf_ptr,
     gnrc_sixlowpan_frag_vrb_t *vrbe = NULL;
 #endif  /* MODULE_GNRC_SIXLOWPAN_FRAG_VRB */
 
+    if (sixlo->size < 2U) {
+        DEBUG("6lo iphc: IPHC header truncated\n");
+        if (rbuf != NULL) {
+            gnrc_sixlowpan_frag_rb_remove(rbuf);
+        }
+        gnrc_pktbuf_release(sixlo);
+        return;
+    }
     if (rbuf != NULL) {
         ipv6 = rbuf->pkt;
         assert(ipv6 != NULL);

diff --git a/sys/net/gnrc/routing/rpl/srh/gnrc_rpl_srh.c b/sys/net/gnrc/routing/rpl/srh/gnrc_rpl_srh.c
@@ -74,6 +74,13 @@ int gnrc_rpl_srh_process(ipv6_hdr_t *ipv6, gnrc_rpl_srh_t *rh, void **err_ptr)
     uint8_t current_pos, pref_elided, addr_len, compri_addr_len;
     const uint8_t new_seg_left = rh->seg_left - 1;
 
+    if ((rh->len * 8) < (GNRC_RPL_SRH_PADDING(rh->pad_resv) +
+                         (16 - GNRC_RPL_SRH_COMPRE(rh->compr)))) {
+        DEBUG("RPL SRH: inconsistent header received\n");
+        *err_ptr = &rh->len;
+        return GNRC_IPV6_EXT_RH_ERROR;
+    }
+
     assert(rh->seg_left > 0);
     num_addr = (((rh->len * 8) - GNRC_RPL_SRH_PADDING(rh->pad_resv) -
                  (16 - GNRC_RPL_SRH_COMPRE(rh->compr))) /

diff --git a/tests/gnrc_rpl_srh/main.c b/tests/gnrc_rpl_srh/main.c
@@ -120,6 +120,27 @@ static void test_rpl_srh_route_multicast(void)
     TEST_ASSERT_NULL(err_ptr);
 }
 
+static void test_rpl_srh_inconsistent_hdr(void)
+{
+    static const ipv6_addr_t dst = IPV6_DST;
+    gnrc_rpl_srh_t srh;
+    void *err_ptr;
+    int res;
+
+    memset(&srh, 0, sizeof(srh));
+    memcpy(&hdr.dst, &dst, sizeof(hdr.dst));
+    srh.nh = 128U;
+    srh.len = 0U;
+    srh.type = 3U;
+    srh.seg_left = 220U;
+    srh.compr = 0xc0;
+    srh.pad_resv = 0xf0;
+
+    res = gnrc_rpl_srh_process(&hdr, &srh, &err_ptr);
+    TEST_ASSERT_EQUAL_INT(res, GNRC_IPV6_EXT_RH_ERROR);
+    TEST_ASSERT((&srh.len) == err_ptr);
+}
+
 static void test_rpl_srh_too_many_seg_left(void)
 {
     static const ipv6_addr_t a1 = IPV6_ADDR1;
@@ -239,6 +260,7 @@ static void run_unittests(void)
     EMB_UNIT_TESTFIXTURES(fixtures) {
         new_TestFixture(test_rpl_srh_dst_multicast),
         new_TestFixture(test_rpl_srh_route_multicast),
+        new_TestFixture(test_rpl_srh_inconsistent_hdr),
         new_TestFixture(test_rpl_srh_too_many_seg_left),
         new_TestFixture(test_rpl_srh_nexthop_no_prefix_elided),
         new_TestFixture(test_rpl_srh_nexthop_prefix_elided),

diff --git a/tests/gnrc_rpl_srh/tests-as-root/01-run.py b/tests/gnrc_rpl_srh/tests-as-root/01-run.py
@@ -167,7 +167,7 @@ def test_wrong_type(child, iface, hw_dst, ll_dst, ll_src):
     pktbuf_empty(child)
 
 
-def test_seg_left_gt_len_addresses(child, iface, hw_dst, ll_dst, ll_src):
+def test_inconsistent_header(child, iface, hw_dst, ll_dst, ll_src):
     # send routing header with no (0) addresses but segleft set to a value
     # larger than 0
     p = srp1(Ether(dst=hw_dst) / IPv6(dst=ll_dst, src=ll_src) /
@@ -176,7 +176,7 @@ def test_seg_left_gt_len_addresses(child, iface, hw_dst, ll_dst, ll_src):
     assert(p is not None)
     assert(ICMPv6ParamProblem in p)
     assert(p[ICMPv6ParamProblem].code == 0)     # erroneous header field encountered
-    assert(p[ICMPv6ParamProblem].ptr == 43)     # segleft field
+    assert(p[ICMPv6ParamProblem].ptr == 41)     # len field
     pktbuf_empty(child)
 
 
@@ -348,7 +348,7 @@ def run(func):
                 raise e
 
     run(test_wrong_type)
-    run(test_seg_left_gt_len_addresses)
+    run(test_inconsistent_header)
     run(test_multicast_dst)
     run(test_multicast_addr)
     run(test_multiple_addrs_of_mine_uncomp)