Buffer overflow in ImGui::Combo()

[tpoechtrager]

ImGui::Combo("Test type", &test_type, "Single call to TextUnformatted()\0Multiple calls to Text(), clipped manually\0Multiple calls to Text(), not clipped");

in imgui_demo.cpp should be (some other calls look buggy too):

ImGui::Combo("Test type", &test_type, "Single call to TextUnformatted()\0Multiple calls to Text(), clipped manually\0Multiple calls to Text(), not clipped\0");

You should really consider a more safe separator, like \003 aka ETX.

ASan report:

================================================================
==28942==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0000005d5f32 at pc 0x00000055564b bp 0x7fffbbc10d70 sp 0x7fffbbc10d68
READ of size 1 at 0x0000005d5f32 thread T0
    #0 0x55564a in ImGui::Combo(char const*, int*, char const*, int) /home/thomas/tmp/imgui/examples/opengl_example/../../imgui.cpp:7580:12
    #1 0x5714a9 in ShowExampleAppLongText(bool*) /home/thomas/tmp/imgui/examples/opengl_example/../../imgui_demo.cpp:2002:5
    #2 0x5714a9 in ImGui::ShowTestWindow(bool*) /home/thomas/tmp/imgui/examples/opengl_example/../../imgui_demo.cpp:103
    #3 0x4d0683 in main /home/thomas/tmp/imgui/examples/opengl_example/main.cpp:76:13
    #4 0x7f1851bf060f in __libc_start_main (/usr/lib/libc.so.6+0x2060f)
    #5 0x418f18 in _start (/data/data/tmp/imgui/examples/opengl_example/imgui_example+0x418f18)

0x0000005d5f32 is located 46 bytes to the left of global variable '<string literal>' defined in '../../imgui_demo.cpp:2003:17' (0x5d5f60) of size 36
  '<string literal>' is ascii string 'Buffer contents: %d lines, %d bytes'
0x0000005d5f32 is located 0 bytes to the right of global variable '<string literal>' defined in '../../imgui_demo.cpp:2002:43' (0x5d5ec0) of size 114
SUMMARY: AddressSanitizer: global-buffer-overflow /home/thomas/tmp/imgui/examples/opengl_example/../../imgui.cpp:7580:12 in ImGui::Combo(char const*, int*, char const*, int)
Shadow bytes around the buggy address:
  0x0000800b2b90: 00 02 f9 f9 f9 f9 f9 f9 00 05 f9 f9 f9 f9 f9 f9
  0x0000800b2ba0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 05
  0x0000800b2bb0: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 03
  0x0000800b2bc0: f9 f9 f9 f9 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9
  0x0000800b2bd0: 00 02 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
=>0x0000800b2be0: 00 00 00 00 00 00[02]f9 f9 f9 f9 f9 00 00 00 00
  0x0000800b2bf0: 04 f9 f9 f9 f9 f9 f9 f9 00 07 f9 f9 f9 f9 f9 f9
  0x0000800b2c00: 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0000800b2c10: f9 f9 f9 f9 00 00 00 00 00 00 00 03 f9 f9 f9 f9
  0x0000800b2c20: 00 00 00 00 00 00 00 07 f9 f9 f9 f9 00 00 00 00
  0x0000800b2c30: 00 00 00 01 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  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
==28942==ABORTING

[ocornut]

Thanks, fixed.

The separator can't really be changed anymore unfortunately. I'll add that idea to my "severe breakage to consider for 2.0" list. Furthermore the semantic meaning of 003/ETX is arcane ascii knowledge and for most people it would feel particularly random/arbitrary. Whereas \0 even if arbitrary is easy to remember.