Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check list length in GrowPlist #2039

Closed
wants to merge 3 commits into from
Closed

Check list length in GrowPlist #2039

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b32bdc
Add functions to check if a C int fits in an immediate GAP int
ChrisJefferson Dec 20, 2017
394727c
Add bounds checking to GrowPlist
ChrisJefferson Dec 20, 2017
6b07c8d
fixup! Add bounds checking to GrowPlist
fingolfin Dec 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/intobj.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,26 @@ static inline Int INT_INTOBJ(Obj o)
#endif
}

/****************************************************************************
**
*F INT_FITS_IN_INTOBJ( <i> ) . check if a C integer fits in an immediate
** integer.
*/
static inline Int INT_FITS_IN_INTOBJ(Int i)
{
return (-(((Int)1) << NR_SMALL_INT_BITS) <= i) &&
(i < ((Int)1) << NR_SMALL_INT_BITS);
}

/****************************************************************************
**
*F UINT_FITS_IN_INTOBJ( <i> ) . check if a C unsigned integer fits in an
** immediate integer.
*/
static inline Int UINT_FITS_IN_INTOBJ(UInt i)
{
return i < ((UInt)1) << NR_SMALL_INT_BITS;
}

/****************************************************************************
**
Expand All @@ -121,6 +141,7 @@ static inline Int INT_INTOBJ(Obj o)
static inline Obj INTOBJ_INT(Int i)
{
Obj o;
GAP_ASSERT(INT_FITS_IN_INTOBJ(i));
o = (Obj)(((UInt)i << 2) + 0x01);
GAP_ASSERT(INT_INTOBJ(o) == i);
return o;
Expand Down
5 changes: 5 additions & 0 deletions src/plist.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ Int GrowPlist (
UInt plen; /* new physical length */
UInt good; /* good new physical length */

if (!UINT_FITS_IN_INTOBJ(need)) {
ErrorMayQuit("GrowPlist: List size too large", 0, 0);
}
Copy link
Member

@fingolfin fingolfin Dec 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is actually not enough: A few lines below, we also "round" up the current capacity of the list, i.e.

    good = 5 * CAPACITY_PLIST(list) / 4 + 4;

So if the list was close to the limit before, then good will exceed the limit -- and then, even if need is below the limit, we end up using good (which is larger than need) as the new list length.

I think the correct solution then is to first check here; then compute good, and change

    if ( need < good ) { plen = good; }
    else               { plen = need; }

to something like

    if ( need < good && UINT_FITS_IN_INTOBJ(good) )
        plen = good;
    else
        plen = need;



/* find out how large the plain list should become */
good = 5 * (SIZE_OBJ(list)/sizeof(Obj)-1) / 4 + 4;

Expand Down