Skip to content

Commit 23d90c8

Browse files
author
Remi Delmas
committed
Fixed possible pointer overflow in CAR upperbound when the object size is zero.
The expression for the upper bound was ub = lb + size - 1 which could overflow when size == 0. It is now set to ub = lb + size. Adding tests that check that CAR instrumentation works with object size of zero and __CPROVER_max_malloc_size.
1 parent cc8321e commit 23d90c8

File tree

5 files changed

+91
-2
lines changed

5 files changed

+91
-2
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#include <stdlib.h>
2+
3+
// returns the index at which the write was performed if any
4+
// -1 otherwise
5+
int foo(char *a, int size)
6+
// clang-format off
7+
__CPROVER_requires(0 <= size && size <= __CPROVER_max_malloc_size)
8+
__CPROVER_requires(a == NULL || __CPROVER_is_fresh(a, size))
9+
__CPROVER_assigns(__CPROVER_POINTER_OBJECT(a))
10+
__CPROVER_ensures(
11+
a && __CPROVER_return_value >= 0 ==> a[__CPROVER_return_value] == 0)
12+
// clang-format on
13+
{
14+
if(!a)
15+
return -1;
16+
int i;
17+
if(0 <= i && i < size)
18+
{
19+
a[i] = 0;
20+
return i;
21+
}
22+
return -1;
23+
}
24+
25+
int main()
26+
{
27+
size_t size;
28+
char *a;
29+
foo(a, size);
30+
return 0;
31+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CORE
2+
main.c
3+
--enforce-contract foo _ --malloc-may-fail --malloc-fail-null
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
^\[foo\.\d+\] line 19 Check that a\[\(signed long (long )?int\)i\] is assignable: SUCCESS
7+
^VERIFICATION SUCCESSFUL$
8+
--
9+
This test checks that objects of size zero or __CPROVER_max_malloc_size
10+
do not cause spurious falsifications in assigns clause instrumentation
11+
in contract enforcement mode.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#include <stdlib.h>
2+
3+
// returns the index at which the write was performed if any
4+
// -1 otherwise
5+
int foo(char *a, int size)
6+
// clang-format off
7+
__CPROVER_requires(0 <= size && size <= __CPROVER_max_malloc_size)
8+
__CPROVER_requires(a == NULL || __CPROVER_rw_ok(a, size))
9+
__CPROVER_assigns(__CPROVER_POINTER_OBJECT(a))
10+
__CPROVER_ensures(
11+
a && __CPROVER_return_value >= 0 ==> a[__CPROVER_return_value] == 0)
12+
// clang-format on
13+
{
14+
if(!a)
15+
return -1;
16+
int i;
17+
if(0 <= i && i < size)
18+
{
19+
a[i] = 0;
20+
return i;
21+
}
22+
return -1;
23+
}
24+
25+
int main()
26+
{
27+
int size;
28+
if(size < 0)
29+
size = 0;
30+
if(size > __CPROVER_max_malloc_size)
31+
size = __CPROVER_max_malloc_size;
32+
char *a = malloc(size * sizeof(*a));
33+
int res = foo(a, size);
34+
if(a && res >= 0)
35+
__CPROVER_assert(a[res] == 0, "expecting SUCCESS");
36+
return 0;
37+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CORE
2+
main.c
3+
--replace-call-with-contract foo _ --malloc-may-fail --malloc-fail-null
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
\[main\.assertion\.1\] line 35 expecting SUCCESS: SUCCESS$
7+
^VERIFICATION SUCCESSFUL$
8+
--
9+
This test checks that objects of size zero or of __CPROVER_max_malloc_size
10+
do not cause spurious falsifications in assigns clause instrumentation
11+
in contract replacement mode.

src/goto-instrument/contracts/assigns.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,7 @@ assigns_clauset::conditional_address_ranget::generate_snapshot_instructions()
133133

134134
instructions.add(goto_programt::make_assignment(
135135
upper_bound_address_var,
136-
minus_exprt{plus_exprt{slice.first, slice.second},
137-
from_integer(1, slice.second.type())},
136+
plus_exprt{slice.first, slice.second},
138137
location_overflow_check));
139138
instructions.destructive_append(skip_program);
140139

0 commit comments

Comments
 (0)