From 12513c0363278eb601ebe8c3d0b7b90562afcb06 Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Sun, 31 May 2026 22:23:29 +0000 Subject: [PATCH] simtool/builder: guard NULL default_param in network tool commands A network client controls the nwc_tool_t default_param, and an empty wire string deserialises to NULL (rdwr_str / plainstring). The change_*/rename/ recolour tool inits and the way/roadsign/building name lookups assume the single-player invariant that the GUI always supplies a non-NULL param, so the empty param reaches atoi(NULL) / *p / desc_table.get(NULL) derefs. Guard the NULL-default_param derefs in those tool inits and make the desc-lookup helpers NULL-safe (way_builder, roadsign, hausbauer), matching the existing bridge/tunnel guards. This is the per-handler (callee) counterpart to the single-point boundary fix on command-null-source-fix, scoped to the same default_param == NULL crash set. The lookup-returned-NULL derefs (off-map pos, unknown building name) and the all-whitespace *p++ over-read are out of scope here. --- src/simutrans/builder/hausbauer.cc | 3 +++ src/simutrans/builder/wegbauer.cc | 3 +++ src/simutrans/obj/roadsign.h | 2 +- src/simutrans/tool/simtool.cc | 27 +++++++++++++++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/simutrans/builder/hausbauer.cc b/src/simutrans/builder/hausbauer.cc index 1bc93c894..7e031e66e 100644 --- a/src/simutrans/builder/hausbauer.cc +++ b/src/simutrans/builder/hausbauer.cc @@ -793,6 +793,9 @@ const building_tile_desc_t *hausbauer_t::find_tile(const char *name, int org_idx if (org_idx < 0) { return NULL; } + if (name == NULL) { + return NULL; + } const building_desc_t *desc = desc_table.get(name); diff --git a/src/simutrans/builder/wegbauer.cc b/src/simutrans/builder/wegbauer.cc index 8e381febc..1007f351e 100644 --- a/src/simutrans/builder/wegbauer.cc +++ b/src/simutrans/builder/wegbauer.cc @@ -240,6 +240,9 @@ bool way_builder_t::waytype_available( const waytype_t wtyp, uint16 time ) const way_desc_t *way_builder_t::get_desc(const char * way_name, const uint16 time) { //DBG_MESSAGE("way_builder_t::get_desc","return desc for %s in (%i)",way_name, time/12); + if( way_name == NULL ) { + return NULL; + } const way_desc_t *desc = desc_table.get(way_name); if( desc && desc->is_available(time) ) { return desc; diff --git a/src/simutrans/obj/roadsign.h b/src/simutrans/obj/roadsign.h index 14beb9073..a0854a52d 100644 --- a/src/simutrans/obj/roadsign.h +++ b/src/simutrans/obj/roadsign.h @@ -209,7 +209,7 @@ class roadsign_t : public obj_t static const roadsign_desc_t *roadsign_search(roadsign_desc_t::types flag, const waytype_t wt, const uint16 time); - static const roadsign_desc_t *find_desc(const char *name) { return table.get(name); } + static const roadsign_desc_t *find_desc(const char *name) { return name ? table.get(name) : NULL; } static const vector_tpl& get_available_signs(const waytype_t wt); }; diff --git a/src/simutrans/tool/simtool.cc b/src/simutrans/tool/simtool.cc index 16db9eaca..ab2a7b269 100644 --- a/src/simutrans/tool/simtool.cc +++ b/src/simutrans/tool/simtool.cc @@ -7813,6 +7813,10 @@ bool tool_change_convoi_t::init( player_t *player ) char tool=0; uint16 convoi_id = 0; + if( default_param == NULL ) { + return false; + } + // skip the rest of the command const char *p = default_param; while( *p && *p<=' ' ) { @@ -7972,6 +7976,10 @@ bool tool_change_line_t::init( player_t *player ) { uint16 line_id = 0; + if( default_param == NULL ) { + return false; + } + // skip the rest of the command const char *p = default_param; while( *p && *p<=' ' ) { @@ -8211,6 +8219,10 @@ bool tool_change_depot_t::init( player_t *player ) sint8 z; uint16 convoi_id; + if( default_param == NULL ) { + return false; + } + // skip the rest of the command const char *p = default_param; while( *p && *p<=' ' ) { @@ -8497,6 +8509,9 @@ bool tool_change_traffic_light_t::init( player_t *player ) koord pos2d; sint8 z; uint16 ns, ticks; + if( default_param == NULL ) { + return false; + } if( 5!=sscanf( default_param, "%hi,%hi,%hhi,%hu,%hu", &pos2d.x, &pos2d.y, &z, &ns, &ticks ) ) { return false; } @@ -8557,6 +8572,9 @@ bool tool_change_city_t::init( player_t *player ) } koord k; sint16 allow_growth; + if( default_param == NULL ) { + return false; + } if( 3!=sscanf( default_param, "g%hi,%hi,%hi", &k.x, &k.y, &allow_growth ) ) { return false; } @@ -8591,6 +8609,9 @@ bool tool_rename_t::init(player_t *player) // skip the rest of the command const char *p = default_param; + if( p == NULL ) { + return false; + } const char what = *p++; switch( what ) { case 'h': @@ -8712,6 +8733,9 @@ bool tool_rename_t::init(player_t *player) bool tool_change_permission_t::init(player_t *player) { uint16 id = 0, perms = 0; + if( default_param == NULL ) { + return false; + } const char *p = default_param; id = atoi(p); @@ -8728,6 +8752,9 @@ bool tool_change_permission_t::init(player_t *player) bool tool_recolour_t::init(player_t *) { + if( default_param == NULL ) { + return false; + } // skip the rest of the command const char *p = default_param; const char what = *p++;