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

Submitted by:  Tom Evans <tom_evans>
Submitted on:  Wed 19 Dec 2007 01:32:31 AM UTC  
 
Category:  None Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  Frédéric Bernon <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>
Project 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>
Project MemberIn charge of this item.
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>
Project 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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project 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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>

 

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
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    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.5