Skip to content

Commit

Permalink
Disallow negative indices in all array access patterns (#138)
Browse files Browse the repository at this point in the history
Disallow negative array indices on all array accesses.
  • Loading branch information
dvander committed Sep 8, 2017
1 parent 12365b3 commit d7743c3
Show file tree
Hide file tree
Showing 15 changed files with 92 additions and 10 deletions.
1 change: 1 addition & 0 deletions compiler/sc.h
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,7 @@ void ffcall(symbol *sym,const char *label,int numargs);
void ffret();
void ffabort(int reason);
void ffbounds(cell size);
void ffbounds();
void jumplabel(int number);
void defstorage(void);
void modstk(int delta);
Expand Down
4 changes: 4 additions & 0 deletions compiler/sc3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2188,10 +2188,14 @@ static int hier1(value *lval1)
if (close==']' && !magic_string) {
if (sym->dim.array.length!=0)
ffbounds(sym->dim.array.length-1); /* run time check for array bounds */
else
ffbounds();
cell2addr(); /* normal array index */
} else {
if (sym->dim.array.length!=0)
ffbounds(sym->dim.array.length*(32/sCHARBITS)-1);
else
ffbounds();
char2addr(); /* character array index */
} /* if */
popreg(sALT);
Expand Down
17 changes: 12 additions & 5 deletions compiler/sc4.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -749,11 +749,18 @@ void ffabort(int reason)

void ffbounds(cell size)
{
if ((sc_debug & sCHKBOUNDS)!=0) {
stgwrite("\tbounds ");
outval(size,TRUE);
code_idx+=opcodes(1)+opargs(1);
} /* if */
stgwrite("\tbounds ");
outval(size,TRUE);
code_idx+=opcodes(1)+opargs(1);
}

void ffbounds()
{
// Since the VM uses an unsigned compare here, this effectively protects us
// from negative array indices.
stgwrite("\tbounds ");
outval(INT_MAX, TRUE);
code_idx += opcodes(1) + opargs(1);
}

/*
Expand Down
7 changes: 7 additions & 0 deletions compiler/tests/fail-index-array-with-const-negative.sp
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
native void printnum(int n);

public main()
{
char x[] = "hello!";
printnum(x[-1]);
}
1 change: 1 addition & 0 deletions compiler/tests/fail-index-array-with-const-negative.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(6) : error 032: array index out of bounds (variable "x")
1 change: 1 addition & 0 deletions tests/exceptions/negative-array-index-1.err
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Error executing main: Array index out-of-bounds (index -1, limit 4)
2 changes: 2 additions & 0 deletions tests/exceptions/negative-array-index-1.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Exception thrown: Array index out-of-bounds (index -1, limit 4)
[0] negative-array-index-1.sp::main, line 8
9 changes: 9 additions & 0 deletions tests/exceptions/negative-array-index-1.sp
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// returnCode: 1
#include <shell>

public main()
{
int x[] = {1, 2, 3, 4};
int n = -1;
printnum(x[n]);
}
1 change: 1 addition & 0 deletions tests/exceptions/negative-array-index-2.err
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Error executing main: Array index out-of-bounds (index -1)
3 changes: 3 additions & 0 deletions tests/exceptions/negative-array-index-2.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Exception thrown: Array index out-of-bounds (index -1)
[0] negative-array-index-2.sp::bad, line 13
[1] negative-array-index-2.sp::main, line 8
14 changes: 14 additions & 0 deletions tests/exceptions/negative-array-index-2.sp
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// returnCode: 1
#include <shell>

public main()
{
int x[] = {1, 2, 3, 4};
int n = -1;
bad(x, n);
}

void bad(int[] x, int n)
{
printnum(x[n]);
}
1 change: 1 addition & 0 deletions tests/exceptions/negative-array-index-3.err
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Error executing main: Array index out-of-bounds (index -1)
3 changes: 3 additions & 0 deletions tests/exceptions/negative-array-index-3.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Exception thrown: Array index out-of-bounds (index -1)
[0] negative-array-index-3.sp::bad, line 13
[1] negative-array-index-3.sp::main, line 8
19 changes: 19 additions & 0 deletions tests/exceptions/negative-array-index-3.sp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// returnCode: 1
#include <shell>

public main()
{
int x[] = {1, 2, 3, 4};
int n = -1;
bad(x, n);
}

void bad(int[] x, int n)
{
bad2(x[n], n);
}

void bad2(int[] x, int n)
{
printnum(x[n]);
}
19 changes: 14 additions & 5 deletions vm/runtime-helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,20 @@ using namespace SourcePawn;
void
ReportOutOfBoundsError(cell_t index, cell_t bounds)
{
Environment::get()->ReportErrorFmt(
SP_ERROR_ARRAY_BOUNDS,
"Array index out-of-bounds (index %d, limit %d)",
index,
size_t(bounds) + 1);
if (bounds == INT_MAX) {
// This is an internal protection against negative indices on arrays with
// unknown size.
Environment::get()->ReportErrorFmt(
SP_ERROR_ARRAY_BOUNDS,
"Array index out-of-bounds (index %d)",
index);
} else {
Environment::get()->ReportErrorFmt(
SP_ERROR_ARRAY_BOUNDS,
"Array index out-of-bounds (index %d, limit %d)",
index,
size_t(bounds) + 1);
}
}

} // namespace sp

0 comments on commit d7743c3

Please sign in to comment.