Skip to content

Commit

Permalink
kernel: validate {CHANGED_,SET_,}METHODS_OPERATION args
Browse files Browse the repository at this point in the history
This ensures we don't return garbage resp. don't trigger an
assertion (in debug builds) when e.g. passing an argument list
with 7 or more entries to ApplicableMethod.
  • Loading branch information
fingolfin committed Dec 28, 2018
1 parent cf47710 commit 17748ef
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 18 deletions.
15 changes: 9 additions & 6 deletions src/opers.c
Original file line number Diff line number Diff line change
Expand Up @@ -3446,8 +3446,9 @@ Obj FuncMETHODS_OPERATION (
Obj meth;

RequireOperation(oper);
RequireNonnegativeSmallInt("METHODS_OPERATION", narg);
n = INT_INTOBJ( narg );
n = IS_INTOBJ(narg) ? INT_INTOBJ(narg) : -1;
if (n < 0 || n >= 7)
RequireArgument("METHODS_OPERATION", narg, "narg", "must be an integer between 0 and 6");
meth = MethsOper( oper, (UInt)n );
#ifdef HPCGAP
MEMBAR_READ();
Expand All @@ -3471,13 +3472,14 @@ Obj FuncCHANGED_METHODS_OPERATION (
Int i;

RequireOperation(oper);
RequireNonnegativeSmallInt("CHANGED_METHODS_OPERATION", narg);
n = IS_INTOBJ(narg) ? INT_INTOBJ(narg) : -1;
if (n < 0 || n >= 7)
RequireArgument("CHANGED_METHODS_OPERATION", narg, "narg", "must be an integer between 0 and 6");
#ifdef HPCGAP
if (!PreThreadCreation) {
ErrorQuit("Methods may only be changed before thread creation",0L,0L);
}
#endif
n = INT_INTOBJ( narg );
cacheBag = CacheOper( oper, (UInt) n );
cache = ADDR_OBJ( cacheBag );
for ( i = 1; i < SIZE_OBJ(cacheBag) / sizeof(Obj); i++ ) {
Expand All @@ -3500,8 +3502,9 @@ Obj FuncSET_METHODS_OPERATION (
Int n;

RequireOperation(oper);
RequireNonnegativeSmallInt("SET_METHODS_OPERATION", narg);
n = INT_INTOBJ( narg );
n = IS_INTOBJ(narg) ? INT_INTOBJ(narg) : -1;
if (n < 0 || n >= 7)
RequireArgument("SET_METHODS_OPERATION", narg, "narg", "must be an integer between 0 and 6");
#ifdef HPCGAP
MEMBAR_WRITE();
#endif
Expand Down
12 changes: 6 additions & 6 deletions src/opers.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ typedef struct {
Obj tester;

// method list of an operation
Obj methods[8];
Obj methods[7];

// cache of an operation
Obj cache[8];
Obj cache[7];

// small integer encoding a set of bit flags with information about the
// operation, see OperExtras below
Expand Down Expand Up @@ -177,13 +177,13 @@ static inline void SET_TESTR_FILT(Obj oper, Obj x)
*/
static inline Obj METHS_OPER(Obj oper, Int i)
{
GAP_ASSERT(0 <= i && i < 8);
GAP_ASSERT(0 <= i && i < 7);
return CONST_OPER(oper)->methods[i];
}

static inline void SET_METHS_OPER(Obj oper, Int i, Obj x)
{
GAP_ASSERT(0 <= i && i < 8);
GAP_ASSERT(0 <= i && i < 7);
OPER(oper)->methods[i] = x;
}

Expand All @@ -194,13 +194,13 @@ static inline void SET_METHS_OPER(Obj oper, Int i, Obj x)
*/
static inline Obj CACHE_OPER(Obj oper, Int i)
{
GAP_ASSERT(0 <= i && i < 8);
GAP_ASSERT(0 <= i && i < 7);
return CONST_OPER(oper)->cache[i];
}

static inline void SET_CACHE_OPER(Obj oper, Int i, Obj x)
{
GAP_ASSERT(0 <= i && i < 8);
GAP_ASSERT(0 <= i && i < 7);
OPER(oper)->cache[i] = x;
}

Expand Down
25 changes: 19 additions & 6 deletions tst/testinstall/kernel/opers.tst
Original file line number Diff line number Diff line change
Expand Up @@ -214,28 +214,41 @@ Error, operation already installed
gap> METHODS_OPERATION(fail,1);
Error, METHODS_OPERATION: <oper> must be an operation (not the value 'fail')
gap> METHODS_OPERATION(Size,-1);
Error, METHODS_OPERATION: <narg> must be a non-negative small integer (not the\
integer -1)
Error, METHODS_OPERATION: <narg> must be an integer between 0 and 6 (not the i\
nteger -1)
gap> METHODS_OPERATION(Size,0);
[ ]
gap> METHODS_OPERATION(Size,6);
[ ]
gap> METHODS_OPERATION(Size,7);
Error, METHODS_OPERATION: <narg> must be an integer between 0 and 6 (not the i\
nteger 7)

# note: CHANGED_METHODS_OPERATION is not usable on HPC-GAP
gap> CHANGED_METHODS_OPERATION(fail,1);
Error, CHANGED_METHODS_OPERATION: <oper> must be an operation (not the value '\
fail')
gap> CHANGED_METHODS_OPERATION(Size,-1);
Error, CHANGED_METHODS_OPERATION: <narg> must be a non-negative small integer \
(not the integer -1)
Error, CHANGED_METHODS_OPERATION: <narg> must be an integer between 0 and 6 (n\
ot the integer -1)
gap> if not IsHPCGAP then CHANGED_METHODS_OPERATION(Size,0); fi;
gap> if not IsHPCGAP then CHANGED_METHODS_OPERATION(Size,6); fi;
gap> CHANGED_METHODS_OPERATION(Size,7);
Error, CHANGED_METHODS_OPERATION: <narg> must be an integer between 0 and 6 (n\
ot the integer 7)

#
gap> SET_METHODS_OPERATION (fail,1,[]);
Error, SET_METHODS_OPERATION: <oper> must be an operation (not the value 'fail\
')
gap> SET_METHODS_OPERATION (Size,-1,[]);
Error, SET_METHODS_OPERATION: <narg> must be a non-negative small integer (not\
the integer -1)
Error, SET_METHODS_OPERATION: <narg> must be an integer between 0 and 6 (not t\
he integer -1)
gap> SET_METHODS_OPERATION (Size,0,[]);
gap> SET_METHODS_OPERATION (Size,6,[]);
gap> SET_METHODS_OPERATION (Size,7,[]);
Error, SET_METHODS_OPERATION: <narg> must be an integer between 0 and 6 (not t\
he integer 7)

#
gap> f:=SETTER_FUNCTION("foobar", IsPGroup);;
Expand Down

0 comments on commit 17748ef

Please sign in to comment.