Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heap overflow in TabBarLayout() #3501

Closed
sleepy-monax opened this issue Oct 1, 2020 · 4 comments
Closed

Heap overflow in TabBarLayout() #3501

sleepy-monax opened this issue Oct 1, 2020 · 4 comments
Labels
bug tabs tab bars, tabs

Comments

@sleepy-monax
Copy link

Version/Branch of Dear ImGui:

Version: current commit
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: glfw/opengl3
Compiler: GCC
Operating System: Linux

My Issue/Question:
There is a heap overflow in TabBarLayout()

static void ImGui::TabBarLayout(ImGuiTabBar* tab_bar)
{
    // ...
    int tab_dst_n = 0;
    // ...
    for (int tab_src_n = 0; tab_src_n < tab_bar->Tabs.Size; tab_src_n++)
    {
        // ...

        ImGuiTabItem* prev_tab = &tab_bar->Tabs[tab_dst_n - 1]; 
        int curr_tab_section_n = (tab->Flags & ImGuiTabItemFlags_Leading) ? 0 : (tab->Flags & ImGuiTabItemFlags_Trailing) ? 2 : 1; // <== HERE tab_dst_n

        // ...
        tab_dst_n++;
    }
}

ASAN output

=================================================================
==82168==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6130000c8294 at pc 0x55b947f65b1c bp 0x7fff2c87fb60 sp 0x7fff2c87fb50
READ of size 4 at 0x6130000c8294 thread T0
    #0 0x55b947f65b1b in TabBarLayout library/imgui/imgui_widgets.cpp:7042
    #1 0x55b947f72e86 in ImGui::TabItemEx(ImGuiTabBar*, char const*, bool*, int, ImGuiWindow*) library/imgui/imgui_widgets.cpp:7584
    #2 0x55b947d90973 in DockNodeUpdateTabBar library/imgui/imgui.cpp:13347
    #3 0x55b947d8aa95 in DockNodeUpdate library/imgui/imgui.cpp:13111
    #4 0x55b947d8b6e9 in DockNodeUpdate library/imgui/imgui.cpp:13137
    #5 0x55b947d8b6e9 in DockNodeUpdate library/imgui/imgui.cpp:13137
    #6 0x55b947d6fb38 in ImGui::DockContextUpdateDocking(ImGuiContext*) library/imgui/imgui.cpp:11998
    #7 0x55b947caf8c9 in ImGui::NewFrame() library/imgui/imgui.cpp:4038
    #8 0x55b947c4f68f in glue::begin_frame() source/glue/Glue.cpp:99
    #9 0x55b947bd0222 in game::GameLoop::run() (/home/nicolas/Cours/Projects/projet-jeu-video/oke-oyibo.out+0x608222)
    #10 0x55b947bc7d96 in main source/main.cpp:37
    #11 0x7f193e583151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)
    #12 0x55b947bc735d in _start (/home/nicolas/Cours/Projects/projet-jeu-video/oke-oyibo.out+0x5ff35d)

0x6130000c8294 is located 20 bytes to the right of 384-byte region [0x6130000c8100,0x6130000c8280)
allocated by thread T0 here:
    #0 0x7f193f5ec459 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x55b947c69632 in MallocWrapper library/imgui/imgui.cpp:928
    #2 0x55b947c98da2 in ImGui::MemAlloc(unsigned long) library/imgui/imgui.cpp:3259
    #3 0x55b947fb19f9 in ImVector<ImGuiTabItem>::reserve(int) library/imgui/imgui.h:1499
    #4 0x55b947fa8f10 in ImVector<ImGuiTabItem>::push_back(ImGuiTabItem const&) library/imgui/imgui.h:1502
    #5 0x55b947f6d46c in ImGui::TabBarAddTab(ImGuiTabBar*, int, ImGuiWindow*) library/imgui/imgui_widgets.cpp:7301
    #6 0x55b947d7c4a8 in DockNodeAddWindow library/imgui/imgui.cpp:12564
    #7 0x55b947db1ced in DockContextBindNodeToWindow library/imgui/imgui.cpp:14789
    #8 0x55b947db26db in ImGui::BeginDocked(ImGuiWindow*, bool*) library/imgui/imgui.cpp:14827
    #9 0x55b947ce0891 in ImGui::Begin(char const*, bool*, int) library/imgui/imgui.cpp:5903
    #10 0x55b947bd06fe in game::GameLoop::display() (/home/nicolas/Cours/Projects/projet-jeu-video/oke-oyibo.out+0x6086fe)
    #11 0x55b947bd010c in game::GameLoop::run()::{lambda()#1}::operator()() const::{lambda()#3}::operator()() const (/home/nicolas/Cours/Projects/projet-jeu-video/oke-oyibo.out+0x60810c)
    #12 0x55b947bd7867 in void debug::Profiler::mesure<game::GameLoop::run()::{lambda()#1}::operator()() const::{lambda()#3}>(game::GameLoop::run()::{lambda()#1}::operator()() const::{lambda()#3}) (/home/nicolas/Cours/Projects/projet-jeu-video/oke-oyibo.out+0x60f867)
    #13 0x55b947bcfea0 in game::GameLoop::run()::{lambda()#1}::operator()() const (/home/nicolas/Cours/Projects/projet-jeu-video/oke-oyibo.out+0x607ea0)
    #14 0x55b947bd8043 in void debug::Profiler::mesure<game::GameLoop::run()::{lambda()#1}>(game::GameLoop::run()::{lambda()#1}) (/home/nicolas/Cours/Projects/projet-jeu-video/oke-oyibo.out+0x610043)
    #15 0x55b947bd02ae in game::GameLoop::run() (/home/nicolas/Cours/Projects/projet-jeu-video/oke-oyibo.out+0x6082ae)
    #16 0x55b947bc7d96 in main source/main.cpp:37
    #17 0x7f193e583151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)

SUMMARY: AddressSanitizer: heap-buffer-overflow library/imgui/imgui_widgets.cpp:7042 in TabBarLayout
Shadow bytes around the buggy address:
  0x0c2680011000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2680011010: 00 00 00 00 00 fa fa fa fa fa fa fa fa fa fa fa
  0x0c2680011020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2680011030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2680011040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c2680011050: fa fa[fa]fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c2680011060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2680011070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2680011080: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
  0x0c2680011090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c26800110a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
  Shadow gap:              cc
==82168==ABORTING
@ocornut
Copy link
Owner

ocornut commented Oct 1, 2020

Hello,

Thank you for reporting!

Can you reproduce this issue in the examples or demo code unmodified?

The ImVector [] operators asserts that the index fits in Size so it is very surprising this would happen.

-Omar

@ocornut ocornut added the tabs tab bars, tabs label Oct 1, 2020
@ocornut
Copy link
Owner

ocornut commented Oct 1, 2020

Never mind, I found the bug. Fixing now. Thank you!

@ocornut ocornut added the bug label Oct 1, 2020
ocornut added a commit that referenced this issue Oct 1, 2020
@ocornut
Copy link
Owner

ocornut commented Oct 1, 2020

Fixed by d02fa93 and merged in Docking.
Thank you for reporting so swiftly.

@ocornut ocornut closed this as completed Oct 1, 2020
ocornut added a commit that referenced this issue Oct 1, 2020
@sleepy-monax
Copy link
Author

You're welcome, thanks for the quick fix 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug tabs tab bars, tabs
Projects
None yet
Development

No branches or pull requests

2 participants