buglwIP - A Lightweight TCP/IP stack - Bugs: bug #22249, TCP MSS set to 0 into TCP option

 
 

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

bug #22249: TCP MSS set to 0 into TCP option

Submitted by:  Marc Chaland <chaland>
Submitted on:  Thu 07 Feb 2008 01:26:43 PM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: Crash ErrorStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: None

(Jump to the original submission Jump to the original submission)

Wed 26 Mar 2008 11:58:39 AM UTC, comment #6:

I have included a test for mss != 0 (in that case, TCP_MSS is used).

I think this fix is good enough and TCP_MSS_MIN is not needed.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 26 Mar 2008 08:21:59 AM UTC, comment #5:

Why not simply test for 0 division? If the remote side wants it to be smaller than 28, why not? Checking for TCP_MSS_MIN would be unnecessary code bloat, in my opinion.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 04 Mar 2008 01:10:21 PM UTC, comment #4:

Lets leave this till after 1.3.0

Kieran Mansley <kieranm>
Project Administrator
Fri 08 Feb 2008 04:00:11 PM UTC, comment #3:

About the RFC, I saw this which provides some context, and indicates that the real minimum can be lower: http://www.ietf.org/mail-archive/web/tsvwg/current/msg04483.html

From that I think we should set the minimum at 28.

Certainly we should be allowing smaller values, as low memory targets such as those running lwip may want them, irrespective of what the link's MTU is capable of.

I'm not sure about making TCP_MSS_MIN an option in lwipopts.h. After all, we should just cope with what the remote size wants as a maximum (the same argument does not apply to TCP_MSS). I think putting it in tcp.h is adequate (maybe surround by ifndef so in practice it can be overridden by those who really care).

I don't know whether changes like this are appropriate for 1.3.0 as they aren't really bug fixes. But it is small and benign. Kieran?

Jonathan Larmour <jifl>
Project Member
Fri 08 Feb 2008 03:34:40 PM UTC, comment #2:

A malicious client may crash easily IP stack by sending a null TCP MSS option. One similar bug has already been fixed :
https://savannah.nongnu.org/bugs/?21494

I don't know if there is a minimum MSS fixed by RFC, but many stacks limit it down to 536.

Marc Chaland <chaland>
Fri 08 Feb 2008 10:00:48 AM UTC, comment #1:

Definetly MSS is not 0,
but I don't know any low limit for it.

Oleg Tyshev <olegreen>
Thu 07 Feb 2008 01:26:43 PM UTC, original submission:

If client tries to connect with a TCP MSS option set to 0, division by 0 exception occurs. Server side should limit minimum size for MSS to 536 into tcp_parseopt() function.

I suggest following changes:

into lwipopts.h, add following define :
#define TCP_MSS_MIN 536
into tcp_in.c replace line 1334 (CVS-1.97) by :
if ((mss < pcb->mss) && (mss > TCP_MSS_MIN)) {
pcb->mss = mss;
}
if (mss < TCP_MSS_MIN) {
pcb->mss = TCP_MSS_MIN;
}

In fact, MSS should not be increased during one connection.

Marc Chaland <chaland>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by kieranm (Posted a comment)
  • -unavailable- added by jifl (Posted a comment)
  • -unavailable- added by olegreen (Posted a comment)
  • -unavailable- added by chaland (Submitted the item)
  •  

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

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 26 Mar 2008 11:58:38 AM UTCgoldsimonStatusNone=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Tue 04 Mar 2008 01:10:21 PM UTCkieranmPlanned Release=>1.3.1

    Back to the top


    Powered by Savane 3.1-cleanup1