gnrc_ipv6: validates payload size

[Yonezawa-T2]

This PR adds validation of

• Payload Size field of IPv6 header,
• Hdr Ext Len fields of IPv6 extension headers, and
• actual payload size >= sizeof(icmpv6_hdr_t).

This PR will protect RIOT from malicious packet trying to access outside of the buffer.

This PR depends on #4654 (merged). I will rebase as soon as #4654 is merged.

[miri64]

I'll wait with my review for #4654 to be merged.

[miri64]

Needs rebase.

[Yonezawa-T2]

Rebased.

[miri64]

Will test at today's Hack'n'ACK

[miri64]

Is not working with NHC (without fragmentation) activated. Strong guess (that's why I tested it with NHC): the decompression marks the UDP header already, making pkt->size shorter than hdr->len here (since it is only the UDP payload, not the full IPv6 payload).

[Yonezawa-T2]

Fixed. Your advice to promote gnrc_pkt_len_upto to a global function was right. Thank you.

[miri64]

Now I have a failed assertion :/

[miri64]

Could you try yourself?

USEMODULE=gnrc_sixlowpan_iphc_nhc make -C examples/gnrc_networking/

and using the udp command.

[Yonezawa-T2]

I forgot to subtract IPv6 header size. Fixed.

BTW, I met an assertion failure in gnrc_ipv6_demux. I will investigate it if possible.

[miri64]

Yes, I'm running into it, too. addr2line reports its at sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c:112, which is confusing, because there is no assert statement in this line.

[Yonezawa-T2]

lldb shows it is the assertion below:

        if (!interested) {
            assert(current == pkt);
            /* IPv6 is not interested anymore so `- 1` */
            receiver_num--;
        }

where pkt and current is:

GNRC_NETTYPE_UDP←pkt
↓next
GNRC_NETTYPE_UDP←current
↓next
GNRC_NETTYPE_IPV6

When I wrote that assertion, I assumed that the function receives only IPv6 headers, extension headers, and unparsed payload but it was wrong.

BTW, below line is also wrong:

    pkt->type = gnrc_nettype_from_protnum(nh);

this should be current->type.

[miri64]

Are you working on a fix for that or should I look into it?

[Yonezawa-T2]

No. Could you look into it? Especially, I'm not sure how to broadcast the packet.

[miri64]

Fixed in #5281. The receiver isn't crashing anymore with both PRs merged, but its still not received properly.

[miri64]

Since this and #5281 change the behavior of GNRC significantly I would prefer to move this to the next release. I'm not sure I manage to work on #5281 until tonight.

[miri64]

#5281 was merged, please rebase.

[Yonezawa-T2]

Rebased.

[miri64]

ACK, please squash.

[miri64]

Wait, does not work, when payload size is >99 (fragmentation threshold).

[miri64]

(I merged current master)

[Yonezawa-T2]

Could you provide some details? tests/gnrc_ipv6_ext modified to use 100 bytes UDP payload seems to be working. (tested on validate_ipv6_size branch rebased on the master (a10151d))

[miri64]

I used gnrc_networking on a board with an IEEE 802.15.4 capable radio (samr21-xpro and iotlab-m3 in this case) and used ping6 <dest> 100. On master this is working, not so on this branch (edit: with master merged in).

[Yonezawa-T2]

Fixed. I also made a 6LoWPAN test with header compression and fragmentation: #5502.

[miri64]

"actual"

[miri64]

Apart from the typo: ACK, please squash.

[Yonezawa-T2]

Fixed typo, squashed.