[lwip-users] Re: [lwip] ip_input bug

[David Haas]

I actually think this code is more or less OK, except I think we should be
testing whether we are waiting for DHCP packets differently.

There is a requirement for a certain size pbuf and for enough of the packet
to be found in the first pbuf at this point. What is the minimum size?
Surely it's large enough for the IP and UDP header to be found. I actually
think the rest of the code won't work if this is not true.

The IP checksum has already been tested here, so I don't see that there is
any risk of accessing undefined memory. If we are accessing beyond the end
of a frame (runt packet), then we are still accessing memory within the pbuf
and thats OK. Remember, the only decision here is whether to pass the packet
up to the transport protocol. The transport protocol and/or DHCP itself can
still reject the packet if it is malformed.

As far as fragmentation goes, I believe it is guarranteed that the transport
header is in the first fragment, although then I don't think this will bring
the other fragments in. Are DHCP packets ever fragmented? That is a possible
problem with this code?


----- Original Message -----
From: "Tim Newsham" <address@hidden>
To: <address@hidden>
Sent: Tuesday, December 17, 2002 8:57 PM
Subject: [lwip] ip_input bug


>
> Some code from ip_input.  There's lots wrong with this:
>
>   /* If a DHCP packet has arrived on the interface, we pass it up the
>      stack regardless of destination IP address. The reason is that
>      DHCP replies are sent to the IP adress that will be given to this
>      node (as recommended by RFC 1542 section 3.1.1, referred by RFC
>      2131). */
>   if(IPH_PROTO(iphdr) == IP_PROTO_UDP &&
>      ((struct udp_hdr *)((u8_t *)iphdr + IPH_HL(iphdr) *
4/sizeof(u8_t)))->src =
> =
>      DHCP_SERVER_PORT) {
>     netif = inp;
>   }
>
>
> 1) accessing the pbuf improperly (is there a requirement that
>    ip_input is called with a pbuf with only 1 buffer on it?
>    It looks like all of ip_input access p->payload directly
>    without checking if there are other buffers in the chain)
> 2) no length check.  If there's a short udp header, this
>    will access undefined parts of memory
> 3) no fragment check.  If this is not the first fragment,
>    its meaningless to look past the IP header w/o reassembly.
>
> seems like a bad way to hack in support for dhcp.
>
> Tim N.
>
> [This message was sent through the lwip discussion list.]

[This message was sent through the lwip discussion list.]