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

[Bug]: Buffer over-read/off-by-one error in StrMakeValid(string_view) #11644

Closed
JGRennison opened this issue Dec 29, 2023 · 0 comments
Closed

Comments

@JGRennison
Copy link
Contributor

Version of OpenTTD

master

Expected result

No bugs

Actual result

When opening a text file window StrMakeValid reads one byte beyond the end of the input buffer because last is inclusive not exclusive.

==2262981==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62800000b963 at pc 0x55f57c07d108 bp 0x7ffc92da4130 sp 0x7ffc92da4120
READ of size 1 at 0x62800000b963 thread T0
    #0 0x55f57c07d107 in StrMakeValid<std::ostreambuf_iterator<char, std::char_traits<char> > > /home/jgr/openttd/trunk/src/string.cpp:118
    #1 0x55f57c076e29 in StrMakeValid[abi:cxx11](std::basic_string_view<char, std::char_traits<char> >, StringValidationSettings) /home/jgr/openttd/trunk/src/string.cpp:219
    #2 0x55f57c1c5b13 in TextfileWindow::LoadText(std::basic_string_view<char, std::char_traits<char> >) /home/jgr/openttd/trunk/src/textfile_gui.cpp:822
    #3 0x55f57c1c5804 in TextfileWindow::LoadTextfile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Subdirectory) /home/jgr/openttd/trunk/src/textfile_gui.cpp:809
    #4 0x55f57b227912 in NewGRFTextfileWindow::NewGRFTextfileWindow(TextfileType, GRFConfig const*) /home/jgr/openttd/trunk/src/newgrf_gui.cpp:564
    #5 0x55f57b20bdbf in ShowNewGRFTextfileWindow(TextfileType, GRFConfig const*) /home/jgr/openttd/trunk/src/newgrf_gui.cpp:579
    #6 0x55f57b237bca in NewGRFWindow::OnClick(Point, int, int) /home/jgr/openttd/trunk/src/newgrf_gui.cpp:948
    #7 0x55f57c87aeb9 in DispatchLeftClickEvent /home/jgr/openttd/trunk/src/window.cpp:733
    #8 0x55f57c8c80d1 in MouseLoop /home/jgr/openttd/trunk/src/window.cpp:2841
    #9 0x55f57c8c9b84 in HandleMouseEvents() /home/jgr/openttd/trunk/src/window.cpp:2938
    #10 0x55f579d7c4c2 in VideoDriver_SDL_Base::PollEvent() /home/jgr/openttd/trunk/src/video/sdl2_v.cpp:417
    #11 0x55f579dbb938 in VideoDriver::Tick() /home/jgr/openttd/trunk/src/video/video_driver.cpp:135
    #12 0x55f579d7fe55 in VideoDriver_SDL_Base::LoopOnce() /home/jgr/openttd/trunk/src/video/sdl2_v.cpp:627
    #13 0x55f579d801d4 in VideoDriver_SDL_Base::MainLoop() /home/jgr/openttd/trunk/src/video/sdl2_v.cpp:645
    #14 0x55f57b551cf9 in openttd_main(int, char**) /home/jgr/openttd/trunk/src/openttd.cpp:817
    #15 0x55f577b608c2 in main /home/jgr/openttd/trunk/src/os/unix/unix_main.cpp:32
    #16 0x7f08d3019d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #17 0x7f08d3019e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #18 0x55f577b606f4 in _start (/home/jgr/openttd/trunk/build-asan/openttd+0x2774d6f4)

0x62800000b963 is located 0 bytes to the right of 14435-byte region [0x628000008100,0x62800000b963)
allocated by thread T0 here:
    #0 0x7f08d635c887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x55f57c1c94d4 in MallocT<char> /home/jgr/openttd/trunk/src/spriteloader/../core/alloc_func.hpp:69
    #2 0x55f57c1c4c86 in TextfileWindow::LoadTextfile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Subdirectory) /home/jgr/openttd/trunk/src/textfile_gui.cpp:776
    #3 0x55f57b227912 in NewGRFTextfileWindow::NewGRFTextfileWindow(TextfileType, GRFConfig const*) /home/jgr/openttd/trunk/src/newgrf_gui.cpp:564
    #4 0x55f57b20bdbf in ShowNewGRFTextfileWindow(TextfileType, GRFConfig const*) /home/jgr/openttd/trunk/src/newgrf_gui.cpp:579
    #5 0x55f57b237bca in NewGRFWindow::OnClick(Point, int, int) /home/jgr/openttd/trunk/src/newgrf_gui.cpp:948
    #6 0x55f57c87aeb9 in DispatchLeftClickEvent /home/jgr/openttd/trunk/src/window.cpp:733
    #7 0x55f57c8c80d1 in MouseLoop /home/jgr/openttd/trunk/src/window.cpp:2841
    #8 0x55f57c8c9b84 in HandleMouseEvents() /home/jgr/openttd/trunk/src/window.cpp:2938
    #9 0x55f579d7c4c2 in VideoDriver_SDL_Base::PollEvent() /home/jgr/openttd/trunk/src/video/sdl2_v.cpp:417
    #10 0x55f579dbb938 in VideoDriver::Tick() /home/jgr/openttd/trunk/src/video/video_driver.cpp:135
    #11 0x55f579d7fe55 in VideoDriver_SDL_Base::LoopOnce() /home/jgr/openttd/trunk/src/video/sdl2_v.cpp:627
    #12 0x55f579d801d4 in VideoDriver_SDL_Base::MainLoop() /home/jgr/openttd/trunk/src/video/sdl2_v.cpp:645
    #13 0x55f57b551cf9 in openttd_main(int, char**) /home/jgr/openttd/trunk/src/openttd.cpp:817
    #14 0x55f577b608c2 in main /home/jgr/openttd/trunk/src/os/unix/unix_main.cpp:32
    #15 0x7f08d3019d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

Steps to reproduce

Open a NewGRF text file window

JGRennison added a commit to JGRennison/Upstream-OpenTTD that referenced this issue Dec 29, 2023
JGRennison added a commit to JGRennison/Upstream-OpenTTD that referenced this issue Dec 29, 2023
JGRennison added a commit to JGRennison/OpenTTD-patches that referenced this issue Dec 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant