Given a partially misaligned memory read for a large number of bytes (e.g., we allocate data at addr [0, 16) but read addr [2, 18)), the address sanitizer (asan) would flag the error as an 'unknown-crash' instead of a 'stack-buffer-overflow' when compiled with gcc.
This is due to a flawed heuristic in asan_errors.cpp, which won't always locate the appropriate shadow byte that would indicate a buffer overflow for any 8 byte or 16+ byte reads. This asan bug impacts gcc, but not clang, as they seem to report errors differently - gcc-compiled binaries report the starting address of the read attempt to asan, while clang-compiled binaries highlight the first byte access that overflows the buffer. This appears to have always been the case at least as far back as gcc-7. This patch resolves this issue via a linear scan of applicable shadow bytes (instead of the original heuristic, which, at best, only increments the shadow byte address by 1 for these scenarios). It also adds appropriate tests to cover for these cases. --- .../c-c++-common/asan/stack-overflow-1.c | 2 ++ .../stack-overflow-multi-byte-partial-1.c | 27 +++++++++++++++++++ .../stack-overflow-multi-byte-partial-2.c | 27 +++++++++++++++++++ .../stack-overflow-multi-byte-partial-3.c | 27 +++++++++++++++++++ .../stack-overflow-multi-byte-partial-4.c | 27 +++++++++++++++++++ .../stack-overflow-multi-byte-partial-5.c | 27 +++++++++++++++++++ .../stack-overflow-multi-byte-partial-6.c | 27 +++++++++++++++++++ .../stack-overflow-multi-byte-partial-7.c | 27 +++++++++++++++++++ .../stack-overflow-multi-byte-partial-8.c | 27 +++++++++++++++++++ libsanitizer/asan/asan_errors.cpp | 7 +++-- 10 files changed, 223 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-1.c create mode 100644 gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-2.c create mode 100644 gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-3.c create mode 100644 gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-4.c create mode 100644 gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-5.c create mode 100644 gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-6.c create mode 100644 gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-7.c create mode 100644 gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-8.c diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c b/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c index 7eab79c6514..c76294b6765 100644 --- a/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c +++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c @@ -17,7 +17,9 @@ int main() { return res; } +/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on address\[^\n\r]*(\n|\r\n|\r)" } */ /* { dg-output "READ of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ /* { dg-output " #0 0x\[0-9a-f\]+ +(in _*main (\[^\n\r]*stack-overflow-1.c:16|\[^\n\r]*:0|\[^\n\r]*\\+0x\[0-9a-z\]*)|\[(\]).*(\n|\r\n|\r)" } */ /* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output ".*Memory access at offset \[0-9\]+ overflows this variable.*(\n|\r\n|\r)" } */ /* { dg-output "\[^\n\r]*in main.*stack-overflow-1.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-1.c b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-1.c new file mode 100644 index 00000000000..8a4fef2be32 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-1.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#define STACK_ALLOC_SIZE 8 +#define READ_SIZE 8 +#define OFFSET 1 + +struct X { + char bytes[READ_SIZE]; +}; + +__attribute__((noinline)) struct X out_of_bounds() { + volatile char bytes[STACK_ALLOC_SIZE]; + struct X* x_ptr = (struct X*)(bytes + OFFSET); + return *x_ptr; +} + +int main() { + struct X x = out_of_bounds(); + return x.bytes[0]; +} + +/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on address\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "READ of size 8 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this variable.*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*in out_of_bounds.*stack-overflow-multi-byte-partial-1.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-2.c b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-2.c new file mode 100644 index 00000000000..ddc86a6ddfc --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-2.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#define STACK_ALLOC_SIZE 8 +#define READ_SIZE 5 +#define OFFSET 5 + +struct X { + char bytes[READ_SIZE]; +}; + +__attribute__((noinline)) struct X out_of_bounds() { + volatile char bytes[STACK_ALLOC_SIZE]; + struct X* x_ptr = (struct X*)(bytes + OFFSET); + return *x_ptr; +} + +int main() { + struct X x = out_of_bounds(); + return x.bytes[0]; +} + +/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on address\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "READ of size 5 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this variable.*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*in out_of_bounds.*stack-overflow-multi-byte-partial-2.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-3.c b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-3.c new file mode 100644 index 00000000000..8f6cf6a1f50 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-3.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#define STACK_ALLOC_SIZE 8 +#define READ_SIZE 8 +#define OFFSET 7 + +struct X { + char bytes[READ_SIZE]; +}; + +__attribute__((noinline)) struct X out_of_bounds() { + volatile char bytes[STACK_ALLOC_SIZE]; + struct X* x_ptr = (struct X*)(bytes + OFFSET); + return *x_ptr; +} + +int main() { + struct X x = out_of_bounds(); + return x.bytes[0]; +} + +/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on address\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "READ of size 8 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this variable.*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*in out_of_bounds.*stack-overflow-multi-byte-partial-3.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-4.c b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-4.c new file mode 100644 index 00000000000..14d989619cc --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-4.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#define STACK_ALLOC_SIZE 16 +#define READ_SIZE 16 +#define OFFSET 7 + +struct X { + char bytes[READ_SIZE]; +}; + +__attribute__((noinline)) struct X out_of_bounds() { + volatile char bytes[STACK_ALLOC_SIZE]; + struct X* x_ptr = (struct X*)(bytes + OFFSET); + return *x_ptr; +} + +int main() { + struct X x = out_of_bounds(); + return x.bytes[0]; +} + +/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on address\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "READ of size 16 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this variable.*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*in out_of_bounds.*stack-overflow-multi-byte-partial-4.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-5.c b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-5.c new file mode 100644 index 00000000000..aafb3ca0f06 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-5.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#define STACK_ALLOC_SIZE 16 +#define READ_SIZE 16 +#define OFFSET 8 + +struct X { + char bytes[READ_SIZE]; +}; + +__attribute__((noinline)) struct X out_of_bounds() { + volatile char bytes[STACK_ALLOC_SIZE]; + struct X* x_ptr = (struct X*)(bytes + OFFSET); + return *x_ptr; +} + +int main() { + struct X x = out_of_bounds(); + return x.bytes[0]; +} + +/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on address\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "READ of size 16 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this variable.*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*in out_of_bounds.*stack-overflow-multi-byte-partial-4.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-6.c b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-6.c new file mode 100644 index 00000000000..c2626e7b706 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-6.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#define STACK_ALLOC_SIZE 16 +#define READ_SIZE 15 +#define OFFSET 2 + +struct X { + char bytes[READ_SIZE]; +}; + +__attribute__((noinline)) struct X out_of_bounds() { + volatile char bytes[STACK_ALLOC_SIZE]; + struct X* x_ptr = (struct X*)(bytes + OFFSET); + return *x_ptr; +} + +int main() { + struct X x = out_of_bounds(); + return x.bytes[0]; +} + +/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on address\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "READ of size 15 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this variable.*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*in out_of_bounds.*stack-overflow-multi-byte-partial-6.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-7.c b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-7.c new file mode 100644 index 00000000000..99f993f708d --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-7.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#define STACK_ALLOC_SIZE 24 +#define READ_SIZE 23 +#define OFFSET 15 + +struct X { + char bytes[READ_SIZE]; +}; + +__attribute__((noinline)) struct X out_of_bounds() { + volatile char bytes[STACK_ALLOC_SIZE]; + struct X* x_ptr = (struct X*)(bytes + OFFSET); + return *x_ptr; +} + +int main() { + struct X x = out_of_bounds(); + return x.bytes[0]; +} + +/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on address\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "READ of size 23 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this variable.*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*in out_of_bounds.*stack-overflow-multi-byte-partial-7.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-8.c b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-8.c new file mode 100644 index 00000000000..b0b700d85c0 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-8.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#define STACK_ALLOC_SIZE 24 +#define READ_SIZE 24 +#define OFFSET 20 + +struct X { + char bytes[READ_SIZE]; +}; + +__attribute__((noinline)) struct X out_of_bounds() { + volatile char bytes[STACK_ALLOC_SIZE]; + struct X* x_ptr = (struct X*)(bytes + OFFSET); + return *x_ptr; +} + +int main() { + struct X x = out_of_bounds(); + return x.bytes[0]; +} + +/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on address\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "READ of size 24 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this variable.*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*in out_of_bounds.*stack-overflow-multi-byte-partial-8.c.*(\n|\r\n|\r)" */ diff --git a/libsanitizer/asan/asan_errors.cpp b/libsanitizer/asan/asan_errors.cpp index 4f112cc5d1b..99facaa8c82 100644 --- a/libsanitizer/asan/asan_errors.cpp +++ b/libsanitizer/asan/asan_errors.cpp @@ -435,8 +435,11 @@ ErrorGeneric::ErrorGeneric(u32 tid, uptr pc_, uptr bp_, uptr sp_, uptr addr, bug_descr = "unknown-crash"; if (AddrIsInMem(addr)) { u8 *shadow_addr = (u8 *)MemToShadow(addr); - // If we are accessing 16 bytes, look at the second shadow byte. - if (*shadow_addr == 0 && access_size > ASAN_SHADOW_GRANULARITY) + u8 *shadow_addr_upper_bound = + shadow_addr + (1 + ((access_size - 1) / ASAN_SHADOW_GRANULARITY)); + // If the read could span multiple shadow bytes, + // do a sequential scan and look for the first bad shadow byte. + while (*shadow_addr == 0 && shadow_addr < shadow_addr_upper_bound) shadow_addr++; // If we are in the partial right redzone, look at the next shadow byte. if (*shadow_addr > 0 && *shadow_addr < 128) shadow_addr++; -- 2.34.1 This e-mail and any attachments may contain information that is confidential and proprietary and otherwise protected from disclosure. If you are not the intended recipient of this e-mail, do not read, duplicate or redistribute it by any means. Please immediately delete it and any attachments and notify the sender that you have received it by mistake. Unintended recipients are prohibited from taking action on the basis of information in this e-mail or any attachments. The DRW Companies make no representations that this e-mail or any attachments are free of computer viruses or other defects.