[lwip-users] What is the correct approach for sanity checks on header fields?

[Martin Sträng]

Hi All,

 

I have some problems with invalid values in both IP and TCP header fields (more specifically the header length field).

When running a test that sends packets with certain invalid values in these fields, our firmware locks up.

The reason for this a LWIP_ERROR check that is trigged in the pbuf_header function which currently result in a LWIP_PLATFORM_ASSERT.

 

This happens for example when the value of header length (in both IP and TCP headers) is larger than the size of the received data (p->len).

 

To me it seems unintended that invalid input packets should result in an assertion.

 

The question is what would be the correct way to fix this. I can see some possibilities:

 

·         One solution would be to  add sanity checks on the header field values involved (for example check that iphdr_len < iphdr_hlen in ip_input and p->len < (hdrlen*4) in tcp_input) to prevent that this situation occurs when later calling pbuf_header.

 

·         Another solution would be to redefine the LWIP_ERROR macro, so that if LWIP_NOASSERT = 1, it only executes the handler (and skips the assertion). This way the erroneous packets would simply be dropped (like they should). The down side is that the LWIP_ERROR macro is used in a lot of places and some of these should arguably  result in an assertion.

 

Ideally I would like to fix this in a way that is consistent with the general design of error checking in lwIP!

 

Thanks // Martin