On Fri, Oct 07, 2022 at 06:00:30PM +0200, [email protected] wrote:
> commit ad4877023146953d4daa8d91c119124c38620337
> Author: Christopher Wellons <[email protected]>
> AuthorDate: Fri Oct 7 11:33:10 2022 -0400
> Commit: Laslo Hunhold <[email protected]>
> CommitDate: Fri Oct 7 18:00:11 2022 +0200
>
> Check for empty destination before NUL-terminating
>
> This overflow was triggered in the second test of to_lowercase_utf8
> where the destination is zero length (w->destlen == 0). `w->destlen`
> would overflow by subtraction, then the subscript would overflow the
> destination.
>
> Signed-off-by: Laslo Hunhold <[email protected]>
>
> diff --git a/src/util.c b/src/util.c
> index ed893ba..2ff1b64 100644
> --- a/src/util.c
> +++ b/src/util.c
> @@ -207,7 +207,7 @@ herodotus_writer_nul_terminate(HERODOTUS_WRITER *w)
> } else { /* w->type == HERODOTUS_TYPE_UTF8 */
> ((char *)(w->dest))[w->first_unwritable_offset] = '\0';
> }
> - } else {
> + } else if (w->destlen > 0) {
> /*
> * In this case, there is no more space in the buffer and
> * the last unwritable offset is larger than
>
I suppose this makes a good argument for wanting to run the tests under
ASan (and UBsan), at least during local development.
One trick I picked up is that `makefile` has higher precedence than
`Makefile`, which means I can do something like the following:
[libgrapheme]~> git checkout 2.0.0
HEAD is now at ef608a2 Bump to version 2.0.0
[libgrapheme]~> make -s clean
[libgrapheme]~> cat makefile
include Makefile
CFLAGS = -O3 -flto=auto -Wall -Wextra -fsanitize=address,undefined -g
LDFLAGS = $(CFLAGS)
[libgrapheme]~> ASAN_OPTIONS=detect_leaks=0 make -j12 -s test
=================================================================
==7679==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7fffd7f302af at pc 0x000000402f50 bp 0x7fffd7f2f990 sp 0x7fffd7f2f988
WRITE of size 1 at 0x7fffd7f302af thread T0
#0 0x402f4f in herodotus_writer_nul_terminate src/util.c:222
#1 0x40e0bd in to_case src/case.c:142
#2 0x40e0bd in grapheme_to_lowercase_utf8 src/case.c:275
#3 0x40e0bd in unit_test_callback_to_case_utf8 test/case.c:529
#4 0x4112a8 in run_unit_tests test/util.c:48
#5 0x401527 in main test/case.c:574
And voila, the overrun gets detected without introducing sanitizer flags
to the Makefile or having to manually specify those flags every time.
A couple notes: `ASAN_OPTIONS=detect_leaks=0` is there to disable the
leak cheker because the LUT generators seems to be leaking memory away.
`-O3 -flto` are there because higher level optimization can also turn on
many warnings which otherwise don't get turned on; LTO in specific can
catch things like parameter type mismatch across TUs which can be rather
difficult to catch otherwise.
- NRK
