Fixed overflow crash in draw_onscreen_direct_messages (#2136)

.github/workflows/PR.yml

@@ -23,8 +23,8 @@ jobs:
     - name: build
       run: |
         set -eux
-        BUILD_NUMBER=3080
-        PACKAGE_SUFFIX= 
+        BUILD_NUMBER=3081
+        PACKAGE_SUFFIX=
         make -j`nproc` BUILD_NUMBER=$BUILD_NUMBER PACKAGE_SUFFIX=$PACKAGE_SUFFIX heavylog DEBUG=1 -k
         make -j`nproc` BUILD_NUMBER=$BUILD_NUMBER PACKAGE_SUFFIX=$PACKAGE_SUFFIX standard
         make -j`nproc` BUILD_NUMBER=$BUILD_NUMBER PACKAGE_SUFFIX=$PACKAGE_SUFFIX package

docs/multiplayer_readme.txt

@@ -4,41 +4,44 @@ Multiplayer in KeeperFX
 KeeperFX supports multiplayer over TCP/IP or ENET/UDP. Serial, Modem and IPX are
 not supported. 1Player allows you to practice multiplayer maps alone.
 
-At the moment of writing, KeeperFX, like Dungeon Keeper, requires a LAN-grade
-connection to work properly. Low latency is what's important, not high speeds.
-Latency is always high when playing over very high distances, so look for local
-friends to play with.
+KeeperFX, like Dungeon Keeper, requires a low latency connection to work properly.
+Latency is always high when playing over very high distances, so playing with 
+people near you will work best. Multiplayer will start to become unplayable when 
+you are over a couple hundred kilometer away from each other.
 
 * High-latency connections will make both games run very slow.
 * Limited to two players.
 * Desyncs and crashes are very possible, report issues and they may be fixed.
 
-Multiplayer over TCP/IP
+Multiplayer over ENET/UDP
 ------------------------------
 
-To host a TCP/IP game:
-Make sure the port 5555 is open for traffic and is forwarded to port 5555 on
-your computer. When you have started the game, click Multiplayer -> TCP/IP ->
+To host a ENET/UDP game:
+Make sure the port 5556 is open for traffic and is forwarded to port 5556 on
+your computer. When you have started the game, click Multiplayer -> ENET/UDP ->
 Create Game.
 
-To join a TCP/IP game:
-Specify a command line option -sessions [ip_address]:5555 when starting game.
+To join a ENET/UDP game:
+Specify a command line option -sessions [ip_address]:5556 when starting game.
 For instance, if the host's IP address is 55.83.54.187, the appropriate command
-line option is -sessions 55.83.54.187:5555
+line option is -sessions 55.83.54.187:5556
 The launcher can be used to set this.
 
 Several sessions can be added to command line by prepending a semicolon before
-each extra address, e.g. -sessions 55.83.54.187:5555;214.43.45.67:5555
+each extra address, e.g. -sessions 55.83.54.187:5555;214.43.45.67:5556
 
-When you have started the game, click Multiplayer -> TCP/IP -> [select IP
+When you have started the game, click Multiplayer -> ENET/UDP -> [select IP
 address in list] -> Join Game.
 
-Multiplayer over ENET/UDP
+Multiplayer over TCP/IP
 ------------------------------
 
-Hosting or joining a game over ENET/UDP works the same as over TCP/IP, with a
-key difference. Open port 5556 instead and select ENET/UDP from the multiplayer
+Hosting or joining a game over TCP/IP works the same as over ENET/UDP, with a
+key difference. Open port 5555 instead and select TCP/IP from the multiplayer
 menu.
 
-+ A faster protocol so could accept a bit more latency for the same lag.
-- Brand new, so totally untested. Go for it and tell us where we're wrong.
+TCP/IP is less suitable for multiplayer as a protocol, and there's no known 
+reason to use this over ENET/UDP as you will have significantly more lag.
+However, this protocal has been around for longer and is no longer being 
+developed or tested on, there's a chance that new bugs on ENET/UDP have
+not ended up in this protocol.

src/frontmenu_saves.c

@@ -112,7 +112,7 @@ void draw_load_button(struct GuiButton *gbtn)
     }
     if (gbtn->content != NULL)
     {
-        sprintf(gui_textbuf, "%s", (const char *)gbtn->content);
+        snprintf(gui_textbuf, sizeof(gui_textbuf), "%s", (const char*)gbtn->content);
         draw_button_string(gbtn, (gbtn->width*32 + 16)/gbtn->height, gui_textbuf);
     }
 }

src/gui_msgs.c

@@ -220,7 +220,7 @@ void message_add(PlayerNumber plyr_idx, const char *text)
 void message_add_va(PlayerNumber plyr_idx, const char *fmt_str, va_list arg)
 {
     static char full_msg_text[2048];
-    vsprintf(full_msg_text, fmt_str, arg);
+    vsnprintf(full_msg_text, sizeof(full_msg_text), fmt_str, arg);
     message_add(plyr_idx, full_msg_text);
 }
 
@@ -237,7 +237,7 @@ void targeted_message_add(PlayerNumber plyr_idx, PlayerNumber target_idx, unsign
     va_list val;
     va_start(val, fmt_str);
     static char full_msg_text[2048];
-    vsprintf(full_msg_text, fmt_str, val);
+    vsnprintf(full_msg_text, sizeof(full_msg_text), fmt_str, val);
     SYNCDBG(2,"Player %d: %s",(int)plyr_idx,full_msg_text);
     for (int i = GUI_MESSAGES_COUNT - 1; i > 0; i--)
     {

src/gui_topmsg.c

@@ -93,6 +93,13 @@ TbBool erstat_check(void)
     // Don't check more often than every 7 turns
     if ((game.play_gameturn & 0x07) != 0)
         return false;
+
+    if (last_checked_stat_num >= sizeof(erstat) / sizeof(erstat[0]))
+    { 
+        ERRORLOG("Invalid last checked stat number %d, resetting to 0", last_checked_stat_num);
+        last_checked_stat_num = 0;
+    }
+
     int stat_num = last_checked_stat_num;
     int sdiff = erstat[stat_num].n - erstat[stat_num].nprv;
     // Display an error if any things were not created in this game turn
@@ -126,25 +133,31 @@ TbBool draw_onscreen_direct_messages(void)
         tx_units_per_px = scale_ui_value_lofi(16);
     }
     // Display in-game message for debug purposes
-    if ((render_onscreen_msg_time > 0.0) || erstat_check())
+    if (erstat_check() || (render_onscreen_msg_time > 0.0))
     {
-        if ( LbScreenIsLocked() )
-      LbTextDrawResized(scale_value_by_horizontal_resolution(160), 0, tx_units_per_px, onscreen_msg_text);
+        if (LbScreenIsLocked())
+        {
+            LbTextDrawResized(scale_value_by_horizontal_resolution(160), 0, tx_units_per_px, onscreen_msg_text);
+        }
         render_onscreen_msg_time -= gameadd.delta_time;
     }
     unsigned int msg_pos = scale_value_by_vertical_resolution(200);
     if ((game.system_flags & GSF_NetGameNoSync) != 0)
     {
         ERRORLOG("OUT OF SYNC (GameTurn %7d)", game.play_gameturn);
-        if ( LbScreenIsLocked() )
-          LbTextDrawResized(scale_value_by_horizontal_resolution(260), scale_value_by_vertical_resolution(msg_pos), tx_units_per_px, "OUT OF SYNC");
+        if (LbScreenIsLocked())
+        {
+            LbTextDrawResized(scale_value_by_horizontal_resolution(260), scale_value_by_vertical_resolution(msg_pos), tx_units_per_px, "OUT OF SYNC");
+        }
         msg_pos += scale_value_by_horizontal_resolution(20);
     }
     if ((game.system_flags & GSF_NetSeedNoSync) != 0)
     {
         ERRORLOG("SEED OUT OF SYNC (GameTurn %7d)", game.play_gameturn);
-        if ( LbScreenIsLocked() )
-          LbTextDrawResized(scale_value_by_horizontal_resolution(260), scale_value_by_vertical_resolution(msg_pos), tx_units_per_px, "SEED OUT OF SYNC");
+        if (LbScreenIsLocked())
+        {
+            LbTextDrawResized(scale_value_by_horizontal_resolution(260), scale_value_by_vertical_resolution(msg_pos), tx_units_per_px, "SEED OUT OF SYNC");
+        }
         msg_pos += scale_value_by_vertical_resolution(20);
     }
     SYNCDBG(18,"Finished");

version.mk

@@ -1,5 +1,5 @@
 VER_MAJOR=0
 VER_MINOR=5
 VER_RELEASE=0
-VER_BUILD=3080
+VER_BUILD=3081
 PACKAGE_SUFFIX=