patchWarzone 2100 Project - Patches: patch #974, Fixes possible stack corruption in...

 
 
Show feedback again

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

patch #974: Fixes possible stack corruption in nettypes.c

Submitted by:  Bugs Buggy <buginator>
Submitted on:  Sun 10 Feb 2008 05:40:35 AM UTC  
 
Category: FixPriority: 9 - Immediate
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
Planned Release: None

Tue 12 Feb 2008 04:28:45 AM UTC, comment #4:

Yes, that is indeed what it should be, ASSERT(FALSE,...)

Bugs Buggy <buginator>
Project Administrator
Sun 10 Feb 2008 11:44:14 AM UTC, comment #3:

Surely you mean ASSERT(FALSE, ...)? It should also never assert on decoding, only on encoding, IMHO.

Per I. Mathisen <per>
Project Administrator
Sun 10 Feb 2008 11:43:09 AM UTC, SVN revision 3733:

Truncate strings and binary data sent over the network if larger than expected.
Patch by Buginator in patch #974 with changes by me.

(Browse SVN revision 3733)

Per I. Mathisen <per>
Project Administrator
Sun 10 Feb 2008 05:42:21 AM UTC, comment #1:

Fixes 2 possible stack corruption cases, when calling function
specifies a length, we should obey their wishes for decoding.
Logging when this happens in LOG_NET debug call.
Alternative for NETstring would be to use a safer function than
memcpy, like perhaps snprintf() ?

Unsure what to do about the case when we specify a length, but the buffer isn't that big for encoding. Perhaps add a assert() ? The only case where this might happen for NETstring() is if the string was "blah\0moreblah\0" Not sure if that can ever happen.

That is patch #1.

All these NET*() calls do not do error reporting, and neither do we check if the NET*() calls fail, so this also needs to be fixed IMO.

For example, NETint8_t(), if it fails, all it does is return NetMsg.status = FALSE;

Now, it does ASSERT(TRUE,"Not enough data/space left in the packet!");

That is patch #2.

(file #3784, file #3785)

Bugs Buggy <buginator>
Project Administrator
Sun 10 Feb 2008 05:40:35 AM UTC, original submission:

Fixes 2 possible stack corruption cases.

Bugs Buggy <buginator>
Project Administrator

 

Attached Files
file #3785:  nettypefixERR.patch added by buginator (2kB - text/x-diff)
file #3784:  nettypefix.patch added by buginator (856B - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by per (Posted a comment)
  • -unavailable- added by buginator (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 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 10 Feb 2008 05:42:21 AM UTCbuginatorAttached File-=>Added nettypefixERR.patch, #3785
    Sun 10 Feb 2008 05:42:20 AM UTCbuginatorAttached File-=>Added nettypefix.patch, #3784
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup