Steps to reproduce:
- Open the line management window
- Select a line and make sure its "Timing data" tab is selected
- Press the Edit button to edit the schedule of the selected line
- Edit the schedule and close the window to apply the changes
This results in a use-after-free because the schedule pointer in gui_times_history_t is not updated when the old schedule is destroyed.
=================================================================
==93588==ERROR: AddressSanitizer: heap-use-after-free on address 0x6030000202d4 at pc 0x555555a8f39f bp 0x7fffffff43d0 sp 0x7fffffff43c8
READ of size 1 at 0x6030000202d4 thread T0
[Detaching after fork from child process 93668]
#0 0x555555a8f39e in schedule_t::matches(karte_t*, schedule_t const*) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x53b39e) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#1 0x5555560525f0 in gui_times_history_t::draw(scr_coord) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0xafe5f0) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#2 0x555555bf32f3 in gui_container_t::draw(scr_coord) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x69f2f3) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#3 0x555555c8b7a2 in gui_scrollpane_t::draw(scr_coord) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x7377a2) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#4 0x555555c9c991 in gui_tab_panel_t::draw(scr_coord) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x748991) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#5 0x555555bf32f3 in gui_container_t::draw(scr_coord) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x69f2f3) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#6 0x555555dcadb6 in gui_frame_t::draw(scr_coord, scr_size) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x876db6) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#7 0x555555faadf6 in schedule_list_gui_t::draw(scr_coord, scr_size) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0xa56df6) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#8 0x555556027d55 in display_all_win() (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0xad3d55) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#9 0x55555602fd86 in win_display_flush(double) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0xadbd86) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#10 0x55555658d1d5 in intr_refresh_display(bool) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x10391d5) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#11 0x5555567063f5 in karte_t::sync_step(unsigned int, bool, bool) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x11b23f5) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#12 0x55555658d34e in interrupt_check(char const*) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x103934e) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#13 0x555556744558 in karte_t::interactive(unsigned int) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x11f0558) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#14 0x5555565b7b11 in simu_main(int, char**) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x1063b11) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#15 0x555556604e79 in sysmain(int, char**) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x10b0e79) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#16 0x7ffff7598d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#17 0x7ffff7598e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#18 0x55555581da04 in _start (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x2c9a04) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
0x6030000202d4 is located 20 bytes inside of 32-byte region [0x6030000202c0,0x6030000202e0)
freed by thread T0 here:
#0 0x5555558dbe7d in operator delete(void*) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x387e7d) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#1 0x555556593ea8 in simline_t::set_schedule(schedule_t*) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x103fea8) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
previously allocated by thread T0 here:
#0 0x5555558db61d in operator new(unsigned long) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x38761d) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#1 0x5555561e6591 in airplane_schedule_::copy() (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0xc92591) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#2 0x5555560e3149 in nwc_tool_t::do_command(karte_t*) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0xb8f149) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#3 0x555556744681 in karte_t::interactive(unsigned int) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x11f0681) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#4 0x5555565b7b11 in simu_main(int, char**) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x1063b11) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#5 0x555556604e79 in sysmain(int, char**) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x10b0e79) (BuildId: fcb55478717b247d932a50e381e81461d37e217e)
#6 0x7ffff7598d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
SUMMARY: AddressSanitizer: heap-use-after-free (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x53b39e) (BuildId: fcb55478717b247d932a50e381e81461d37e217e) in schedule_t::matches(karte_t*, schedule_t const*)
Shadow bytes around the buggy address:
0x0c067fffc000: 03 fa fa fa 00 00 00 fa fa fa 00 00 02 fa fa fa
0x0c067fffc010: 00 00 00 01 fa fa 00 00 00 fa fa fa fa fa fa fa
0x0c067fffc020: fa fa 00 00 00 fa fa fa 00 00 03 fa fa fa fd fd
0x0c067fffc030: fd fd fa fa 00 00 00 00 fa fa fa fa fa fa fa fa
0x0c067fffc040: 00 00 00 06 fa fa fd fd fd fd fa fa fd fd fd fd
=>0x0c067fffc050: fa fa fd fd fd fd fa fa fd fd[fd]fd fa fa fd fd
0x0c067fffc060: fd fd fa fa fa fa fa fa fa fa 00 00 00 02 fa fa
0x0c067fffc070: 00 00 00 03 fa fa fd fd fd fd fa fa 00 00 02 fa
0x0c067fffc080: fa fa 00 00 04 fa fa fa 00 00 00 03 fa fa 00 00
0x0c067fffc090: 00 03 fa fa 00 00 00 fa fa fa 00 00 00 00 fa fa
0x0c067fffc0a0: 00 00 00 00 fa fa fd fd fd fd fa fa fd fd fd fd
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
==93588==ABORTING
Quote from: ceeac on September 15, 2022, 07:52:53 PMSteps to reproduce:
- Open the line management window
- Select a line and make sure its "Timing data" tab is selected
- Press the Edit button to edit the schedule of the selected line
- Edit the schedule and close the window to apply the changes
I'm afraid but I can't reproduce it in my environment.
(https://i.imgur.com/hzwUxL4.gif)
You can see in the image that it updates correctly immediately after updating the schedule.
QuoteThis results in a use-after-free because the schedule pointer in gui_times_history_t is not updated when the old schedule is destroyed.
I can't reproduce the crash so I can't verify a workaround for this.
Can it be avoided by executing init() if schedule is not found in the first line of gui_times_history_t::draw()?
The line management window is one of the older style dialogs that hasn't been updated yet.
Recently updated garage dialog broke as it incorporated commits from the standard until it was updated...
This should be updated in the future. It could be improved if it is implemented.
A similar tab also exists in the convoy dialog. But the convoy dialog has been updated to an aligned container.
It would also be helpful if you could clarify if a similar crash occurs in the convoy dialog as well.
Quote from: Ranran on September 16, 2022, 09:18:02 AMI'm afraid but I can't reproduce it in my environment.
The bug only shows up under ASan because the memory is used immediately after it is freed so the memory contents are still the same as before the destruction of the old schedule.
Recent versions of MSVC support the /fsanitize=address option (see here (https://learn.microsoft.com/en-us/cpp/build/reference/fsanitize?view=msvc-170)) so the bug should show up if the option is enabled.
Quote from: Ranran on September 16, 2022, 09:18:02 AMA similar tab also exists in the convoy dialog. But the convoy dialog has been updated to an aligned container.
It would also be helpful if you could clarify if a similar crash occurs in the convoy dialog as well.
Yes, I can also reproduce the bug with the convoy dialogue.
I have been able to reproduce this after updating Visual Studio and enabling /fsanitize=address as instructed. The problem appears to occur in UI code, however, and I am not familiar with this code.