off-by-one error in -fsanitizer=bounds when addressing a pointer instead of an integral

[kees]

The bounds sanitizer does not trip when accessing the address of the last array element (but does if it accessed as an integral). For example:

#include <stdlib.h>
#include <stdio.h>

#define SIZE 3

struct foo {
int count;
int array[SIZE];
};

volatile int zero = 0; // hide const expression size from optimizer

int main(int argc, char *argv[]) {
int size = SIZE + zero;
// include trailing space to avoid segfaults on "out of bounds" access
struct foo *p = calloc(1, sizeof(*p) + sizeof(int) + sizeof(int));

// this correctly trips sanitizer:
int val = p->array[size];
printf("%d\n", val);

// this does not?!
int *valp = &p->array[size];
printf("%p %d\n", valp, *valp);

// but this does...
int *val2 = &p->array[size + 1];
printf("%p %d\n", val2, *val2);

return 0;

}

Built with: -O2 -Wall -fstrict-flex-arrays=3 -fsanitize=bounds

./example.c:19:23: runtime error: index 3 out of bounds for type 'int [3]'
0
0xd0b42c0 0
./example.c:27:26: runtime error: index 4 out of bounds for type 'int [3]'
0xd0b42c4 0

This was noticed while using the "counted_by" attribute on a flexible array, but it is present even with fixed-size arrays.

[kees]

cc @isanbard @bwendling

[Peter0x44]

This is not a bug.
Having pointers one past the end of an object is allowed in C. This is also how C++ iterators work.

[kees]

I don't debate having pointers past the end of an object -- it is, however, illegal to access an invalid array index under any situation. And if [size] is "allowable" than why is [size + 1] not? And if this is a C++ism, then it needs to be fixed for C only. Note that this isn't a pointer iterator, it's an array. So the indexing must never go out of bounds no matter what the usage.

For example, standard array iteration won't ever attempt it:

for (i = 0; i < max_size; i++) {
  p->array[i] ....
}

And certainly getting a "pointer past the array" is fine, but it has to be done from outside the array, not from within it, i.e. using the object itself as the base reference, not the object's member (which is "inside" the object). These are the same rules as for __builtin_dynamic_object_size(), etc.

void *p = (void *)p + offsetof(typeof(*p), array) + __builtin_dynamic_object_size(p->array, 1);

Otherwise there is no way to unambiguously enforce bounds checking on the array.

[kees]

In the GCC bug, I see why this exception was made:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119132
These still needs to be a mode/flag that makes this unambiguous for the bounds sanitizer, though, for code bases that are willing to accept the change in favor of greater memory safety coverage.

[Peter0x44]

The diagnostic you're seeing has nothing to do with the deference. It is just for the actual calculation of the address itself. Which for the case of &array[size] is well-defined.

#include <stdio.h>

#define SIZE 3

struct foo {
int count;
int array[SIZE];
};

volatile int zero = 0; // hide const expression size from optimizer

int main(int argc, char *argv[]) {
int size = SIZE + zero;
// include trailing space to avoid segfaults on "out of bounds" access
struct foo *p = calloc(1, sizeof(*p) + sizeof(int) + sizeof(int));

// this correctly trips sanitizer:
//int val = p->array[size];
//printf("%d\n", val);

// this does not?!
int *valp = &p->array[size];
//printf("%p %d\n", valp, *valp);

// but this does...
int *val2 = &p->array[size + 1];
//printf("%p %d\n", val2, *val2);

return 0;
}

If I comment the lines doing the deference out of your testcase, it has the same results.

[manuel5975p]

I think the key part is this:

int main(){
    float values[3] = {0.0f, 0.0f, 0.0f};

    float* addr_one_past_end = &values[3]; //Trips no address sanitizer
    float* addr_two_past_end = &values[4]; //Trips all address sanitizers
    return 0;
}