patchGnash - The GNU Flash player - Patches: patch #7533, [PATCH] AbcBlock.h: checkBounds...


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

patch #7533: [PATCH] AbcBlock.h: checkBounds should check for negative values

Submitted by:  Mj Mendoza IV <creek23>
Submitted on:  Tue 26 Apr 2011 04:03:18 AM UTC  
Category:  core Priority:  3 - Low
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open

Wed 27 Apr 2011 08:12:19 AM UTC, comment #3: 

The point is that if size_t can't be negative there's no point in checking for it to be < 0.
The compiler should notice that and warn you with something like: "comparison will always be false". Please check if it does.

If a caller tries to pass a negative number to a function expecting an unsigned type the body of the funcion will see that value reinterpreted as a positive value, which usually gets evaluated as > [maxvalue].

If a check has to be made for negative values it should be made outside of the function, that is while the value is still signed, right after reading it from the stack...

Sandro Santilli <strk>
Project Member
Wed 27 Apr 2011 03:11:52 AM UTC, comment #2: 

I believe so, but I'm only trying to address a TODO inside `libcore/vm/Machine.cpp' at line 64:

/// TODO: it's quite possible for a malformed SWF to ask for out-of-bounds
/// pool access, although at the moment it's mainly Gnash bugs causing this.
/// Throwing an exception is good here, but it's not clear which one.

All `pool_{datatype}' are using `try/catch' to address the above TODO. But each `{datatype}PoolAt' are already using the checkBounds, which is not checking for negative values.

My suggestion is for `checkBounds' to implement checking of negative values so these try/catches will be removed.

Mj Mendoza IV <creek23>
Tue 26 Apr 2011 10:08:40 AM UTC, comment #1: 

Isn't size_t unsigned ?
I'm pretty sure there's a ssize_t for signed type.

Sandro Santilli <strk>
Project Member
Tue 26 Apr 2011 04:03:18 AM UTC, original submission:  

There might be a chance that malformed values could be negative.

Mj Mendoza IV <creek23>


Attached Files
file #23304:  AbcBlock.h.patch added by creek23 (95B - application/octet-stream)
file #23305:  AbcBlock.h.patch.htm added by creek23 (209KiB - text/html)


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -email is unavailable- added by strk (Posted a comment)
  • -email is unavailable- added by creek23 (Submitted the item)

    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 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-04-26 creek23 Attached File- => Added AbcBlock.h.patch, #23304
        Attached File- => Added AbcBlock.h.patch.htm, #23305

    Back to the top

    Powered by Savane 3.5