The International Simutrans Forum


Author Topic: Bug: [master] Crash when clicking 'changing price' on new brig in shipyard.  (Read 311 times)

0 Members and 1 Guest are viewing this topic.

Offline Hanczar

  • *
  • Posts: 28
I have it 100% reproducible.

local build from master branch made today.

Code: [Select]
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff6a75f5d in __GI_abort () at abort.c:90
#2  0x00007ffff6abe28d in __libc_message (action=action@entry=(do_abort | do_backtrace), fmt=fmt@entry=0x7ffff6be3a26 "*** %s ***: %s terminated\n") at ../sysdeps/posix/libc_fatal.c:181
#3  0x00007ffff6b6483f in __GI___fortify_fail_abort (need_backtrace=need_backtrace@entry=true, msg=msg@entry=0x7ffff6be39ad "buffer overflow detected") at fortify_fail.c:33
#4  0x00007ffff6b64861 in __GI___fortify_fail (msg=msg@entry=0x7ffff6be39ad "buffer overflow detected") at fortify_fail.c:44
#5  0x00007ffff6b62550 in __GI___chk_fail () at chk_fail.c:28
#6  0x00007ffff6b61a39 in _IO_str_chk_overflow (fp=<optimized out>, c=<optimized out>) at vsprintf_chk.c:31
#7  0x00007ffff6ac2d59 in __GI__IO_default_xsputn (f=0x7fffffffae80, data=<optimized out>, n=37) at genops.c:455
#8  0x00007ffff6a9099f in _IO_vfprintf_internal (s=s@entry=0x7fffffffae80, format=format@entry=0x555555f15bf0 "Catering income per km (full convoy):", ap=ap@entry=0x7fffffffafc0) at vfprintf.c:1328
#9  0x00007ffff6b61adb in ___vsprintf_chk (s=0x7fffffffb1a0 "Catering income per km (full co", flags=1, slen=32, format=0x555555f15bf0 "Catering income per km (full convoy):", args=args@entry=0x7fffffffafc0) at vsprintf_chk.c:82
#10 0x00007ffff6b61a0a in ___sprintf_chk (s=s@entry=0x7fffffffb1a0 "Catering income per km (full co", flags=flags@entry=1, slen=slen@entry=32, format=<optimized out>) at sprintf_chk.c:31
#11 0x000055555570e73c in sprintf (__fmt=<optimized out>, __s=0x7fffffffb1a0 "Catering income per km (full co") at /usr/include/x86_64-linux-gnu/bits/stdio2.h:34
#12 gui_class_vehicleinfo_t::draw (this=this@entry=0x555578cd0d88, offset=...) at gui/
#13 0x000055555567a45c in gui_scrollpane_t::draw (this=0x555578cd0b18, pos=...) at gui/components/
#14 0x0000555555670c65 in gui_container_t::draw (this=this@entry=0x555578cd0aa8, offset=...) at gui/components/
#15 0x00005555556a687b in gui_frame_t::draw (this=this@entry=0x555578cd0aa0, pos=..., size=...) at gui/
#16 0x000055555570cb80 in vehicle_class_manager_t::draw (this=this@entry=0x555578cd0aa0, pos=..., size=...) at gui/
#17 0x00005555556fe5ea in display_win (win=win@entry=2) at gui/
#18 0x00005555556ff02f in display_all_win () at gui/
#19 0x0000555555701405 in win_display_flush (konto=8099.3999999999996) at gui/
#20 0x00005555557feb00 in intr_refresh_display (dirty=<optimized out>) at
#21 0x0000555555846460 in karte_t::sync_step (this=0x555556881dc0, delta_t=<optimized out>, do_sync_step=<optimized out>, display=display@entry=true) at
#22 0x00005555557fec0e in interrupt_check (caller_info=caller_info@entry=0x5555558dba77 "") at
#23 0x000055555585c48d in karte_t::interactive (this=this@entry=0x555556881dc0, quit_month=quit_month@entry=2147483647) at
#24 0x0000555555807a4c in simu_main (argc=argc@entry=1, argv=argv@entry=0x7fffffffdfc8) at
#25 0x0000555555816872 in sysmain (argc=1, argv=0x7fffffffdfc8) at
#26 0x00007ffff6a5e1c1 in __libc_start_main (main=0x5555555a7fa0 <main(int, char**)>, argc=1, argv=0x7fffffffdfc8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdfb8) at ../csu/libc-start.c:308
#27 0x00005555555a7fda in _start ()

Problem is quite obvious in gui/ - stack buffer overflow.
Code: [Select]
sprintf(catering_service, translator::translate("catering_income_pr_km_(full_convoy):"));
catering_service  is allocated as 32 bytes buffer and above string wrote to it is longer.

My proposed fix which works for me (buffer size increase + safe way of writing strings to local buffer):
Code: [Select]
git diff ./
diff --git a/gui/ b/gui/
index 0b16d7e93..f7f4ecd9e 100644
--- a/gui/
+++ b/gui/
@@ -1019,15 +1019,15 @@ void gui_class_vehicleinfo_t::draw(scr_coord offset)
                                if (v->get_desc()->get_catering_level() > 0)
                                        uint32 unit_count = 0;
-                                       char catering_service[32];
+                                       char catering_service[64];
                                        if (mail_veh)
-                                               sprintf(catering_service, translator::translate("tpo_income_pr_km_(full_convoy):"));
+                                               snprintf(catering_service,sizeof(catering_service), "%s",translator::translate("tpo_income_pr_km_(full_convoy):"));
                                                unit_count = mail_count;
-                                               sprintf(catering_service, translator::translate("catering_income_pr_km_(full_convoy):"));
+                                               snprintf(catering_service,sizeof(catering_service), "%s",translator::translate("catering_income_pr_km_(full_convoy):"));
                                                unit_count = passenger_count;
                                        extra_y += LINESPACE;

Offline jamespetts gb

  • Simutrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 18030
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Thank you very much for that - that is most helpful. I have now incorporated the first, main part of the fix.

I have not incorporated the second part of the fix, however, as I have had problems with "snprintf" compiling in Visual Studio in the past, and I use Visual Studio builds for debugging. Hopefully, the first part of the fix should solve this in any event.