buglwIP - A Lightweight TCP/IP stack - Bugs: bug #21846, LwIP doesn't appear to perform any...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #21846: LwIP doesn't appear to perform any IP Source Address Filtering

Submitter:  Tom Evans <tom_evans>
Submitted:  Wed 19 Dec 2007 01:32:31 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  fbernon
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

Jump to the original submission

Thu 03 Jan 2008 05:50:01 PM UTC, comment #9: 

Done as check_ip_src.

Jonathan Larmour <jifl>
Group Member
Thu 03 Jan 2008 05:13:48 PM UTC, comment #8: 

I have a preference for check_ip_src (without _valid), but change it if you prefer.

Frédéric Bernon <fbernon>
Group Member
Thu 03 Jan 2008 02:36:18 PM UTC, comment #7: 

I've been away so only just looking at this. In comment #5 are you asking if people think ipsrcchk is a good variable name? If so, then I think check_ip_src_valid would be clearer. If you agree, I'll change (so small I'd normally just do it to save effort, but I don't want to tread on toes).

Jonathan Larmour <jifl>
Group Member
Sun 30 Dec 2007 01:10:16 AM UTC, comment #6: 

Check in. Please, reopen if something is wrong...

Frédéric Bernon <fbernon>
Group Member
Mon 24 Dec 2007 06:16:26 PM UTC, comment #5: 

ipsrcchk = have to check "ip source". But when I read ip_src_valid, I think to "the ip source is valid".

Do you have another suggestion?


Frédéric Bernon <fbernon>
Group Member
Sun 23 Dec 2007 04:13:04 PM UTC, comment #4: 

From looking at the patch file (can't test at the moment), it looks fine. Although I would change the variable name to something more meaningful like ip_src_valid.

Simon Goldschmidt <goldsimon>
Group administrator
Sun 23 Dec 2007 12:07:47 PM UTC, comment #3: 

Add an exception in the patch for dhcp packets.

(file #14690)

Frédéric Bernon <fbernon>
Group Member
Sun 23 Dec 2007 11:44:52 AM UTC, comment #2: 

Sorry, there is an extra pbuf_free in the patch. I attach a new one.

(file #14689)

Frédéric Bernon <fbernon>
Group Member
Sat 22 Dec 2007 10:57:40 PM UTC, comment #1: 

Seems a valid comment. Proposed patch file is attached. Any objections?


(file #14685)

Frédéric Bernon <fbernon>
Group Member
Wed 19 Dec 2007 01:32:31 AM UTC, original submission:  

RFC1122 (October 1989):
ftp://ftp.rfc-editor.org/in-notes/rfc1122.txt
Section 3.2.1.3

"A host MUST silently discard an incoming datagram
containing an IP source address that is invalid by
the rules of this section.  This validation could
be done in either the IP layer or by each protocol
in the transport layer."

By "this section" above they mean Multicast or Broadcast.

I can't find anything in ip.c that performs this check. There are no apparent checks on the source address in there, udp.c or tcp_input.c.

The test might be as simple as the following, but I haven't checked this at all, I'm assuming the broadcast test code is appropriate:

/* Broadcast or Multicast Source address, RFC 1122: 3.2.1.3 */
if ((ip_addr_isbroadcast(&(iphdr->src), inp)) ||
    (ip_addr_ismulticast(&(iphdr->src)))) {
    ... debug print, free buffer, count stats, return ERR ...
}

It should probably go in ip.c after "#endif /* LWIP_DHCP */" as I think DHCP can use strange source addresses.



Tom Evans <tom_evans>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #14690:  ip.c.patch added by fbernon (2KiB - application/octet-stream)
file #14689:  ip.c.patch added by fbernon (933B - application/octet-stream)
file #14685:  ip.c.patch added by fbernon (953B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by fbernon (Updated the item)
  • -email is unavailable- added by tom_evans (Submitted the item)
  • -email is unavailable- added by tom_evans
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-12-30 fbernon Item GroupNone Faulty Behaviour
        StatusNone Fixed
        Assigned toNone fbernon
        Open/ClosedOpen Closed
        Planned Release 1.3.0
    2007-12-23 fbernon Attached File- Added ip.c.patch, #14690
    2007-12-23 fbernon Attached File- Added ip.c.patch, #14689
    2007-12-22 fbernon Attached File- Added ip.c.patch, #14685
    2007-12-19 tom_evans Carbon-Copy- Added tom_evans

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code