Typing non-ASCII character causes heap-buffer-overflow

[matthiakl]

When I try to enter non-ASCII characters (e.g. üöä) AddressSanitizer terminates my SDL application with heap-buffer-overflow. With a debugger attached I found that X11_XLookupString already returned UTF-8, so the conversion created a not-terminated string.
This is a regression from #4699, if I revert this commit it works fine.

Backtrace

==266062==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000251594 at pc 0x7ffff79998ef bp 0x7fffffff3c40 sp 0x7fffffff33e8
READ of size 5 at 0x602000251594 thread T0
    #0 0x7ffff79998ee in __interceptor_strlen /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:461
    #1 0x7ffff5398fd9 in SDL_strlen_REAL **/SDL/src/stdlib/SDL_string.c:345
    #2 0x7ffff539922e in SDL_strlcpy_REAL **/SDL/src/stdlib/SDL_string.c:548
    #3 0x7ffff5522daf in XLookupStringAsUTF8 **/SDL/src/video/x11/SDL_x11events.c:748
    #4 0x7ffff5523ada in X11_DispatchEvent **/SDL/src/video/x11/SDL_x11events.c:1100
    #5 0x7ffff55251d9 in X11_PumpEvents **/SDL/src/video/x11/SDL_x11events.c:1719
    #6 0x7ffff531eaa4 in SDL_PumpEventsInternal **/SDL/src/events/SDL_events.c:922
    #7 0x7ffff531eea5 in SDL_WaitEventTimeout_REAL **/SDL/src/events/SDL_events.c:1097
    #8 0x7ffff531eb88 in SDL_PollEvent_REAL **/SDL/src/events/SDL_events.c:964
    #9 0x7ffff5310a68 in SDL_PollEvent **/SDL/src/dynapi/SDL_dynapi_procs.h:156
...
0x602000251594 is located 0 bytes after 4-byte region [0x602000251590,0x602000251594)

allocated by thread T0 here:

#0 0x7ffff79f9369 in __interceptor_malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69

#1 0x7ffff5398292 in real_malloc **/SDL/src/stdlib/SDL_malloc.c:5196

#2 0x7ffff53984f9 in SDL_malloc_REAL **/SDL/src/stdlib/SDL_malloc.c:5295

#3 0x7ffff53980d8 in SDL_iconv_string_REAL **/SDL/src/stdlib/SDL_iconv.c:814

#4 0x7ffff5522d8b in XLookupStringAsUTF8 **/SDL/src/video/x11/SDL_x11events.c:746

#5 0x7ffff5523ada in X11_DispatchEvent **/SDL/src/video/x11/SDL_x11events.c:1100

#6 0x7ffff55251d9 in X11_PumpEvents **/SDL/src/video/x11/SDL_x11events.c:1719

#7 0x7ffff531eaa4 in SDL_PumpEventsInternal **/SDL/src/events/SDL_events.c:922

#8 0x7ffff531eea5 in SDL_WaitEventTimeout_REAL **/SDL/src/events/SDL_events.c:1097

#9 0x7ffff531eb88 in SDL_PollEvent_REAL **/SDL/src/events/SDL_events.c:964

#10 0x7ffff5310a68 in SDL_PollEvent **/SDL/src/dynapi/SDL_dynapi_procs.h:156

...
SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:461 in __interceptor_strlen

Shadow bytes around the buggy address:

0x602000251300: fa fa fd fa fa fa 00 fa fa fa 00 fa fa fa fd fa

0x602000251380: fa fa 00 fa fa fa 00 fa fa fa fd fa fa fa fd fd

0x602000251400: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd

0x602000251480: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd

0x602000251500: fa fa fd fd fa fa fd fa fa fa fd fd fa fa fd fa

=>0x602000251580: fa fa[04]fa fa fa fa fa fa fa fa fa fa fa fa fa

0x602000251600: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa

0x602000251680: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa

0x602000251700: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa

0x602000251780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa

0x602000251800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa

Shadow byte legend (one shadow byte represents 8 application bytes):

Addressable:           00

Partially addressable: 01 02 03 04 05 06 07

Heap left redzone:       fa

Freed heap region:       fd

Stack left redzone:      f1

Stack mid redzone:       f2

Stack right redzone:     f3

Stack after return:      f5

Stack use after scope:   f8

Global redzone:          f9

Global init order:       f6

Poisoned by user:        f7

Container overflow:      fc

Array cookie:            ac

Intra object redzone:    bb

ASan internal:           fe

Left alloca redzone:     ca

Right alloca redzone:    cb

==266062==ABORTING

System Information:

• Arch Linux (Cinnamon x11)
• SDL2 version 7f7ea909b
• localectl:

System Locale: LANG=de_DE.UTF-8
    VC Keymap: neo
   X11 Layout: de
    X11 Model: neo

[slouken]

@matthiakl, do you get the same AddressSanitizer result from this code?

    {
        const char *string = "ü";
        char *utf8_text = SDL_iconv_string("UTF-8", "ISO-8859-1", string, SDL_strlen(string));
        if (utf8_text) {
            printf("Result: %s\n", utf8_text);
            SDL_free(utf8_text);
        }
    }

I get the result: ü

[slouken]

There are a few weird things here:
First, SDL_strlcpy() only calls SDL_strlen() if HAVE_STRLCPY isn't defined. I would assume that would be available on a Linux system?
Second, SDL_iconv_string() always allocates at least 5 bytes for the result, and always null terminates the string.
Third, even if we interpret ü encoded in UTF-8 as Latin-1, and convert that to UTF-8, it'll always succeed, encoding that as 4 bytes.

Can you debug in and see where the call is failing, and why that would result in a buffer overrun?

[matthiakl]

do you get the same AddressSanitizer result from this code?

Yes, it still results in a heap buffer overflow, but the string content before the crash is also "ü".

First, SDL_strlcpy() only calls SDL_strlen() if HAVE_STRLCPY isn't defined. I would assume that would be available on a Linux system?

It looks like it is part of libbsd in my system and not automatically linked in:

$ man 3 strlcpy
LIBRARY
     Utility functions from BSD systems (libbsd, -lbsd)

Second, SDL_iconv_string() always allocates at least 5 bytes for the result, and always null terminates the string.

This is only true for the main branch, on SDL2 branch it only allocates at least 4 bytes:

SDL/src/stdlib/SDL_iconv.c

Line 814 in bfef7c3

string = (char *)SDL_malloc(stringsize);

Applying 5f5abb6 solved the issue: It now longer crashes and in the textbox the correct character appears (it got converted correctly somewhere else?). So please backport it to SDL2.