From f6cef421cfe66e19bbed3abe7b1b50f4bb02bb45 Mon Sep 17 00:00:00 2001
From: Jan Rydzewski <SupraSummus@users.noreply.github.com>
Date: Thu, 28 May 2026 22:14:15 +0000
Subject: [PATCH] pak loader: bounds-check descriptor reads

Tightens malformed-pak handling so the loader fails with a
diagnostic instead of walking past the buffer.

- obj_desc_t stores nchildren; get_child<T>(i) dbg->fatals on
  out-of-bounds i.  Pins a NULL-deref reachable via
  skin_reader_t::register_obj on a 0-child MENU node.

- node_body wraps one node's fread'd bytes in a bounds-checked
  cursor that carries the reader's type name.  Each reader's
  read_node starts with
    auto p = node_body(fp, node.size, get_type_name());
    if (!p) return NULL;
  and the existing decode_*(p) calls keep working via free
  decode_uint*(node_body&) overloads.  The raw-pointer
  decode_uint*(char*&) overloads stay; pakset_manager.cc still
  uses them for the 5 node-header reads.  Fatal on overrun
  reports offset / total / need / have and the reader's name.

- read_node_info caps a large-record node.size at remaining
  file bytes so a truncated pak doesn't drive a multi-GB
  malloc.  obj_named_desc_t::get_name picks up the same
  NULL-check the sibling get_copyright already has;
  register_desc<> skips when get_name returns NULL.

One gap not closed here: image_t::alloc(decode_uint32(p)) still
takes its length straight from the buffer.  Same
allocation-size class as the read_node_info cap but at the
reader level.  Happy to follow up if useful.
---
 src/simutrans/dataobj/pakset_manager.cc       |  17 +++
 src/simutrans/descriptor/obj_base_desc.h      |   3 +-
 src/simutrans/descriptor/obj_desc.h           |  16 ++-
 .../descriptor/reader/bridge_reader.cc        |   8 +-
 .../descriptor/reader/building_reader.cc      |  15 +--
 .../descriptor/reader/citycar_reader.cc       |   8 +-
 .../descriptor/reader/crossing_reader.cc      |   8 +-
 .../descriptor/reader/factory_reader.cc       |  45 ++------
 .../descriptor/reader/good_reader.cc          |   8 +-
 .../descriptor/reader/groundobj_reader.cc     |   8 +-
 .../descriptor/reader/image_reader.cc         |  17 ++-
 .../descriptor/reader/imagelist2d_reader.cc   |   8 +-
 .../descriptor/reader/imagelist_reader.cc     |   8 +-
 src/simutrans/descriptor/reader/obj_reader.h  | 107 ++++++++++++++++++
 .../descriptor/reader/pedestrian_reader.cc    |   8 +-
 .../descriptor/reader/roadsign_reader.cc      |   8 +-
 .../descriptor/reader/sound_reader.cc         |  10 +-
 .../descriptor/reader/tree_reader.cc          |   8 +-
 .../descriptor/reader/tunnel_reader.cc        |   6 +-
 .../descriptor/reader/vehicle_reader.cc       |   8 +-
 .../descriptor/reader/way_obj_reader.cc       |   8 +-
 src/simutrans/descriptor/reader/way_reader.cc |   8 +-
 .../descriptor/reader/xref_reader.cc          |  20 ++--
 src/simutrans/descriptor/spezial_obj_tpl.h    |   6 +-
 24 files changed, 202 insertions(+), 164 deletions(-)

diff --git a/src/simutrans/dataobj/pakset_manager.cc b/src/simutrans/dataobj/pakset_manager.cc
index d6d7d2dd69..c7ea089e1f 100644
--- a/src/simutrans/dataobj/pakset_manager.cc
+++ b/src/simutrans/dataobj/pakset_manager.cc
@@ -333,6 +333,22 @@ static bool read_node_info(obj_node_info_t& node, FILE* const f, uint32 const ve
 			return false;
 		}
 		node.size = decode_uint32(p);
+
+		// Reject a node whose body would extend past EOF, before any
+		// reader allocates a buffer sized to it.  The small-record
+		// path is uint16-bounded and can't OOM; only the large path
+		// needs this guard.
+		const long pos = ftell(f);
+		if (pos >= 0 && fseek(f, 0, SEEK_END) == 0) {
+			const long end = ftell(f);
+			if (fseek(f, pos, SEEK_SET) != 0) {
+				return false;
+			}
+			if (end >= pos && node.size > (uint32)(end - pos)) {
+				dbg->error("read_node_info", "node size %u exceeds remaining file bytes %ld", node.size, end - pos);
+				return false;
+			}
+		}
 	}
 
 	return true;
@@ -358,6 +374,7 @@ bool pakset_manager_t::read_nodes(FILE *fp, obj_desc_t *&data, int node_depth, u
 
 		if (node.nchildren != 0) {
 			data->children = new obj_desc_t *[node.nchildren];
+			data->nchildren = node.nchildren;
 
 			for (int i = 0; i < node.nchildren; i++) {
 				if (!read_nodes(fp, data->children[i], node_depth + 1, version)) {
diff --git a/src/simutrans/descriptor/obj_base_desc.h b/src/simutrans/descriptor/obj_base_desc.h
index d895275958..e927129c6a 100644
--- a/src/simutrans/descriptor/obj_base_desc.h
+++ b/src/simutrans/descriptor/obj_base_desc.h
@@ -24,7 +24,8 @@ class obj_named_desc_t : public obj_desc_t
 public:
 	const char *get_name() const
 	{
-		return get_child<text_desc_t>(0)->get_text();
+		const text_desc_t *const ts = get_child<text_desc_t>(0);
+		return ts ? ts->get_text() : NULL;
 	}
 
 	const char *get_copyright() const
diff --git a/src/simutrans/descriptor/obj_desc.h b/src/simutrans/descriptor/obj_desc.h
index c2a9be62ce..cfe63d2d65 100644
--- a/src/simutrans/descriptor/obj_desc.h
+++ b/src/simutrans/descriptor/obj_desc.h
@@ -8,6 +8,7 @@
 
 
 #include <cstddef>
+#include "../simdebug.h"
 #include "../simtypes.h"
 
 /**
@@ -19,7 +20,7 @@ class obj_desc_t
 	friend class pakset_manager_t;
 
 public:
-	obj_desc_t() : children() {}
+	obj_desc_t() : children(), nchildren() {}
 
 	~obj_desc_t() { delete [] children; }
 
@@ -44,14 +45,17 @@ class obj_desc_t
 	}
 
 protected:
-	template<typename T> T const* get_child(int const i) const { return static_cast<T const*>(children[i]); }
+	template<typename T> T const* get_child(unsigned const i) const
+	{
+		if (i >= nchildren) {
+			dbg->fatal("obj_desc_t::get_child", "requested child %u of %u", i, nchildren);
+		}
+		return static_cast<T const*>(children[i]);
+	}
 
 private:
-	/*
-	 * Internal Node information - the derived class knows,
-	 * how many node child nodes really exist.
-	 */
 	obj_desc_t** children;
+	uint16 nchildren;
 
 	friend class factory_field_group_reader_t;
 	friend class obj_reader_t;
diff --git a/src/simutrans/descriptor/reader/bridge_reader.cc b/src/simutrans/descriptor/reader/bridge_reader.cc
index b5135be35e..6c8f012921 100644
--- a/src/simutrans/descriptor/reader/bridge_reader.cc
+++ b/src/simutrans/descriptor/reader/bridge_reader.cc
@@ -12,7 +12,6 @@
 #include "bridge_reader.h"
 #include "../obj_node_info.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 #include <inttypes.h>
 #include <stdio.h>
@@ -32,11 +31,8 @@ void bridge_reader_t::register_obj(obj_desc_t *&data)
 
 obj_desc_t *bridge_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/building_reader.cc b/src/simutrans/descriptor/reader/building_reader.cc
index dc62422e41..11c8f0378b 100644
--- a/src/simutrans/descriptor/reader/building_reader.cc
+++ b/src/simutrans/descriptor/reader/building_reader.cc
@@ -12,7 +12,6 @@
 #include "../obj_node_info.h"
 #include "building_reader.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 /**
@@ -33,11 +32,8 @@ struct old_btyp
 
 obj_desc_t * tile_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the highest bit was always cleared.
@@ -218,11 +214,8 @@ bool building_reader_t::successfully_loaded() const
 
 obj_desc_t *building_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char * p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the highest bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/citycar_reader.cc b/src/simutrans/descriptor/reader/citycar_reader.cc
index 68821a789e..c40c04a340 100644
--- a/src/simutrans/descriptor/reader/citycar_reader.cc
+++ b/src/simutrans/descriptor/reader/citycar_reader.cc
@@ -16,7 +16,6 @@
 
 #include "../../simdebug.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 void citycar_reader_t::register_obj(obj_desc_t *&data)
@@ -39,11 +38,8 @@ bool citycar_reader_t::successfully_loaded() const
 
 obj_desc_t * citycar_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/crossing_reader.cc b/src/simutrans/descriptor/reader/crossing_reader.cc
index 032e32ab8f..91e3f462a3 100644
--- a/src/simutrans/descriptor/reader/crossing_reader.cc
+++ b/src/simutrans/descriptor/reader/crossing_reader.cc
@@ -15,7 +15,6 @@
 
 #include "../../simdebug.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 void crossing_reader_t::register_obj(obj_desc_t *&data)
@@ -33,11 +32,8 @@ void crossing_reader_t::register_obj(obj_desc_t *&data)
 
 obj_desc_t * crossing_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/factory_reader.cc b/src/simutrans/descriptor/reader/factory_reader.cc
index 4a4d89a608..211141353a 100644
--- a/src/simutrans/descriptor/reader/factory_reader.cc
+++ b/src/simutrans/descriptor/reader/factory_reader.cc
@@ -12,7 +12,6 @@
 #include "../factory_desc.h"
 #include "../xref_desc.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 #include "factory_reader.h"
 
@@ -41,11 +40,8 @@ uint16 rescale_probability(const uint16 p)
 
 obj_desc_t *factory_field_class_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	uint16 v = decode_uint16(p);
 	field_class_desc_t *desc = new field_class_desc_t();
@@ -74,11 +70,8 @@ obj_desc_t *factory_field_class_reader_t::read_node(FILE *fp, obj_node_info_t &n
 
 obj_desc_t *factory_field_group_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	uint16 v = decode_uint16(p);
 	field_group_desc_t *desc = new field_group_desc_t();
@@ -168,11 +161,8 @@ void factory_field_group_reader_t::register_obj(obj_desc_t *&data)
 
 obj_desc_t *factory_smoke_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	sint16 x = decode_sint16(p);
 	sint16 y = decode_sint16(p);
@@ -194,16 +184,11 @@ obj_desc_t *factory_smoke_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 
 obj_desc_t *factory_supplier_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
-
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
-
 	const uint16 v = decode_uint16(p);
 	const int version = v & 0x8000 ? v & 0x7FFF : 0;
 
@@ -232,11 +217,8 @@ obj_desc_t *factory_supplier_reader_t::read_node(FILE *fp, obj_node_info_t &node
 
 obj_desc_t *factory_product_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
@@ -270,11 +252,8 @@ obj_desc_t *factory_product_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 
 obj_desc_t *factory_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/good_reader.cc b/src/simutrans/descriptor/reader/good_reader.cc
index 1a23f29e3a..5c23c2d953 100644
--- a/src/simutrans/descriptor/reader/good_reader.cc
+++ b/src/simutrans/descriptor/reader/good_reader.cc
@@ -11,7 +11,6 @@
 #include "../obj_node_info.h"
 #include "../goods_desc.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 void goods_reader_t::register_obj(obj_desc_t *&data)
@@ -37,11 +36,8 @@ bool goods_reader_t::successfully_loaded() const
 
 obj_desc_t * goods_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/groundobj_reader.cc b/src/simutrans/descriptor/reader/groundobj_reader.cc
index f535169142..5869a365ff 100644
--- a/src/simutrans/descriptor/reader/groundobj_reader.cc
+++ b/src/simutrans/descriptor/reader/groundobj_reader.cc
@@ -15,7 +15,6 @@
 #include "../obj_node_info.h"
 #include "groundobj_reader.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 #include <cinttypes>
 
@@ -45,11 +44,8 @@ bool groundobj_reader_t::successfully_loaded() const
 
 obj_desc_t *groundobj_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the highest bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/image_reader.cc b/src/simutrans/descriptor/reader/image_reader.cc
index 6df9ade7eb..de76c24121 100644
--- a/src/simutrans/descriptor/reader/image_reader.cc
+++ b/src/simutrans/descriptor/reader/image_reader.cc
@@ -16,7 +16,6 @@
 
 #include <zlib.h>
 #include "../../tpl/inthashtable_tpl.h"
-#include "../../tpl/array_tpl.h"
 
 
 // if without graphics backend, do not copy any pixel
@@ -28,16 +27,14 @@
 
 obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin()+6;
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
-	// always zero in old version, since length was always less than 65535
-	// because a node could not hold more data
+	// version byte sits at offset 6; old versions stored 0 there
+	// since the node size header was uint16 and couldn't reach 65535.
+	p.seek(6);
 	uint8 version = decode_uint8(p);
-	p = desc_buf.begin();
+	p.seek(0);
 
 #if COLOUR_DEPTH != 0
 	image_t *desc = new image_t();
@@ -60,7 +57,7 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 		//PAKSET_INFO("image_t::read_node()","x,y=%d,%d  w,h=%d,%d, len=%i",desc->x,desc->y,desc->w,desc->h, desc->len);
 
 		uint16* dest = desc->data;
-		p = desc_buf.begin()+12;
+		p.seek(12);
 
 		if (desc->h > 0) {
 			for (uint i = 0; i < desc->len; i++) {
diff --git a/src/simutrans/descriptor/reader/imagelist2d_reader.cc b/src/simutrans/descriptor/reader/imagelist2d_reader.cc
index 8487bd6726..0a594594db 100644
--- a/src/simutrans/descriptor/reader/imagelist2d_reader.cc
+++ b/src/simutrans/descriptor/reader/imagelist2d_reader.cc
@@ -10,16 +10,12 @@
 
 #include "imagelist2d_reader.h"
 #include "../obj_node_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 obj_desc_t * imagelist2d_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	image_array_t *desc = new image_array_t();
 	desc->count = decode_uint16(p);
diff --git a/src/simutrans/descriptor/reader/imagelist_reader.cc b/src/simutrans/descriptor/reader/imagelist_reader.cc
index a5ca577627..1338e1426b 100644
--- a/src/simutrans/descriptor/reader/imagelist_reader.cc
+++ b/src/simutrans/descriptor/reader/imagelist_reader.cc
@@ -10,16 +10,12 @@
 
 #include "imagelist_reader.h"
 #include "../obj_node_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 obj_desc_t * imagelist_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	image_list_t *desc = new image_list_t();
 	desc->count = decode_uint16(p);
diff --git a/src/simutrans/descriptor/reader/obj_reader.h b/src/simutrans/descriptor/reader/obj_reader.h
index 8893b42a1e..db093a7c31 100644
--- a/src/simutrans/descriptor/reader/obj_reader.h
+++ b/src/simutrans/descriptor/reader/obj_reader.h
@@ -8,6 +8,7 @@
 
 
 #include <stdio.h>
+#include <memory>
 
 #include "../obj_node_info.h"
 #include "../objversion.h"
@@ -96,6 +97,112 @@ inline uint64 decode_uint64(char *&data)
 		static classname the_instance
 
 
+/// Owns the bytes of one pak node body plus a bounds-checked cursor
+/// over them.  Constructed in each reader's read_node() with
+/// `auto p = node_body(fp, node.size, get_type_name());`;
+/// decode_*(node_body&) free overloads dispatch to its bounds-checked
+/// readers.
+class node_body
+{
+public:
+	/// Allocate @p size bytes, fread them from @p fp.  Short fread
+	/// leaves the cursor in failure state (bool-false).
+	/// @p size == 0 yields a valid but empty cursor.
+	node_body(FILE* fp, size_t size, const char* type_name)
+		: buf_(new char[size > 0 ? size : 1]),
+		  pos_(NULL),
+		  end_(NULL),
+		  type_name_(type_name)
+	{
+		if (size > 0 && fread(buf_.get(), size, 1, fp) != 1) {
+			buf_.reset();
+			return;
+		}
+		pos_ = buf_.get();
+		end_ = buf_.get() + size;
+	}
+
+	/// false on short fread.
+	explicit operator bool() const { return pos_ != NULL; }
+
+	/// Bounds-checked absolute seek from the buffer start.
+	void seek(size_t off)
+	{
+		if (buf_.get() + off > end_) {
+			dbg->fatal(type_name_,
+				"seek to offset %zu past buffer end %zu",
+				off, (size_t)(end_ - buf_.get()));
+		}
+		pos_ = buf_.get() + off;
+	}
+
+	/// Bounds-checked skip-forward; pairs with the legacy
+	/// `p += N` idiom at reader call sites.
+	node_body& operator+=(size_t n) { require(n); pos_ += n; return *this; }
+
+	/// Single-byte skip — `p++` for callers that used to advance a
+	/// raw char* by one byte (typically to step over a version tag).
+	/// Returns *this rather than a pre-increment copy because the
+	/// owned unique_ptr buffer can't be cheaply duplicated; no current
+	/// caller reads the return value.
+	node_body& operator++(int) { require(1); ++pos_; return *this; }
+
+	/// Return the current cursor and advance by @p n bytes
+	/// (bounds-checked).  Used for memcpy-style tail reads.
+	char* read_bytes(size_t n) { require(n); char* p = pos_; pos_ += n; return p; }
+
+	uint8 read_uint8()   { require(1); return (uint8)*pos_++; }
+	uint16 read_uint16() { require(2); uint16 v = (uint16)(uint8)pos_[0] | (uint16)(uint8)pos_[1] << 8; pos_ += 2; return v; }
+	uint32 read_uint32()
+	{
+		require(4);
+		uint32 v =
+			(uint32)(uint8)pos_[0]       |
+			(uint32)(uint8)pos_[1] <<  8 |
+			(uint32)(uint8)pos_[2] << 16 |
+			(uint32)(uint8)pos_[3] << 24;
+		pos_ += 4;
+		return v;
+	}
+	uint64 read_uint64()
+	{
+		require(8);
+		uint64 v = 0;
+		for (int i = 0; i < 8; ++i) {
+			v |= (uint64)(uint8)pos_[i] << (i * 8);
+		}
+		pos_ += 8;
+		return v;
+	}
+
+private:
+	void require(size_t n) const
+	{
+		if (pos_ + n > end_) {
+			dbg->fatal(type_name_,
+				"short read at offset %zu of %zu: need %zu, have %zu",
+				(size_t)(pos_ - buf_.get()), (size_t)(end_ - buf_.get()),
+				n, (size_t)(end_ - pos_));
+		}
+	}
+
+	std::unique_ptr<char[]> buf_;
+	char* pos_;
+	char* end_;
+	const char* type_name_;
+};
+
+/// Free decode_uint*() overloads that dispatch to the cursor's
+/// bounds-checked readers.  Paired with the free char*& overloads
+/// above: `decode_uint16(p)` resolves to either based on whether
+/// `p` is a node_body (reader bodies) or char* (pakset_manager
+/// node-header reads).
+inline uint8  decode_uint8(node_body& b)  { return b.read_uint8(); }
+inline uint16 decode_uint16(node_body& b) { return b.read_uint16(); }
+inline uint32 decode_uint32(node_body& b) { return b.read_uint32(); }
+inline uint64 decode_uint64(node_body& b) { return b.read_uint64(); }
+
+
 class obj_reader_t
 {
 public:
diff --git a/src/simutrans/descriptor/reader/pedestrian_reader.cc b/src/simutrans/descriptor/reader/pedestrian_reader.cc
index 7eccbde7bc..878d005a8d 100644
--- a/src/simutrans/descriptor/reader/pedestrian_reader.cc
+++ b/src/simutrans/descriptor/reader/pedestrian_reader.cc
@@ -12,7 +12,6 @@
 
 #include "pedestrian_reader.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 void pedestrian_reader_t::register_obj(obj_desc_t *&data)
@@ -39,11 +38,8 @@ bool pedestrian_reader_t::successfully_loaded() const
  */
 obj_desc_t * pedestrian_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/roadsign_reader.cc b/src/simutrans/descriptor/reader/roadsign_reader.cc
index 48f48731de..b53af7e0c0 100644
--- a/src/simutrans/descriptor/reader/roadsign_reader.cc
+++ b/src/simutrans/descriptor/reader/roadsign_reader.cc
@@ -15,7 +15,6 @@
 
 #include "../../simdebug.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 #include <cinttypes>
 
@@ -40,11 +39,8 @@ bool roadsign_reader_t::successfully_loaded() const
 
 obj_desc_t *roadsign_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	const uint16 v = decode_uint16(p);
 	const int version = v & 0x8000 ? v & 0x7FFF : 0;
diff --git a/src/simutrans/descriptor/reader/sound_reader.cc b/src/simutrans/descriptor/reader/sound_reader.cc
index 304ae7d7f6..608e045fc6 100644
--- a/src/simutrans/descriptor/reader/sound_reader.cc
+++ b/src/simutrans/descriptor/reader/sound_reader.cc
@@ -11,7 +11,6 @@
 #include "../obj_node_info.h"
 
 #include "../../simdebug.h"
-#include "../../tpl/array_tpl.h"
 
 
 void sound_reader_t::register_obj(obj_desc_t *&data)
@@ -25,11 +24,8 @@ void sound_reader_t::register_obj(obj_desc_t *&data)
 
 obj_desc_t * sound_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	const uint16 v = decode_uint16(p);
 	const int version = v & 0x8000 ? v & 0x7FFF : 0;
@@ -45,7 +41,7 @@ obj_desc_t * sound_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 		desc->nr = decode_uint16(p);
 		uint16 len = decode_uint16(p);
 		if(  len>0  ) {
-			desc->nr = desc->get_sound_id(p);
+			desc->nr = desc->get_sound_id(p.read_bytes(len));
 		}
 	}
 	else {
diff --git a/src/simutrans/descriptor/reader/tree_reader.cc b/src/simutrans/descriptor/reader/tree_reader.cc
index 5f7c065af5..4dff03a4a9 100644
--- a/src/simutrans/descriptor/reader/tree_reader.cc
+++ b/src/simutrans/descriptor/reader/tree_reader.cc
@@ -13,7 +13,6 @@
 #include "../obj_node_info.h"
 #include "tree_reader.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 void tree_reader_t::register_obj(obj_desc_t *&data)
@@ -35,11 +34,8 @@ bool tree_reader_t::successfully_loaded() const
 
 obj_desc_t * tree_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the highest bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/tunnel_reader.cc b/src/simutrans/descriptor/reader/tunnel_reader.cc
index 0573776cea..45b8937d2f 100644
--- a/src/simutrans/descriptor/reader/tunnel_reader.cc
+++ b/src/simutrans/descriptor/reader/tunnel_reader.cc
@@ -17,7 +17,6 @@
 
 #include "../../builder/tunnelbauer.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 #include <cinttypes>
 
@@ -67,12 +66,11 @@ obj_desc_t * tunnel_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 		return desc;
 	}
 
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) {
 		delete desc;
 		return NULL;
 	}
-	char *p = desc_buf.begin();
 
 	const uint16 v = decode_uint16(p);
 	const uint16 version = v & 0x8000 ? v & 0x7FFF : 0;
diff --git a/src/simutrans/descriptor/reader/vehicle_reader.cc b/src/simutrans/descriptor/reader/vehicle_reader.cc
index cc90b7d682..5492403ba3 100644
--- a/src/simutrans/descriptor/reader/vehicle_reader.cc
+++ b/src/simutrans/descriptor/reader/vehicle_reader.cc
@@ -14,7 +14,6 @@
 #include "vehicle_reader.h"
 #include "../obj_node_info.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 void vehicle_reader_t::register_obj(obj_desc_t *&data)
@@ -37,11 +36,8 @@ bool vehicle_reader_t::successfully_loaded() const
 
 obj_desc_t *vehicle_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/way_obj_reader.cc b/src/simutrans/descriptor/reader/way_obj_reader.cc
index e897c15543..a1e59ccace 100644
--- a/src/simutrans/descriptor/reader/way_obj_reader.cc
+++ b/src/simutrans/descriptor/reader/way_obj_reader.cc
@@ -13,7 +13,6 @@
 #include "way_obj_reader.h"
 #include "../obj_node_info.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 #include <cinttypes>
 
@@ -37,11 +36,8 @@ bool way_obj_reader_t::successfully_loaded() const
 
 obj_desc_t * way_obj_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
-	char *p = desc_buf.begin();
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
 	// old versions of PAK files have no version stamp.
 	// But we know, the higher most bit was always cleared.
diff --git a/src/simutrans/descriptor/reader/way_reader.cc b/src/simutrans/descriptor/reader/way_reader.cc
index 7b1290f9a6..ce8123624c 100644
--- a/src/simutrans/descriptor/reader/way_reader.cc
+++ b/src/simutrans/descriptor/reader/way_reader.cc
@@ -14,7 +14,6 @@
 #include "way_reader.h"
 #include "../obj_node_info.h"
 #include "../../network/pakset_info.h"
-#include "../../tpl/array_tpl.h"
 
 
 void way_reader_t::register_obj(obj_desc_t *&data)
@@ -38,12 +37,9 @@ bool way_reader_t::successfully_loaded() const
 
 obj_desc_t * way_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	array_tpl<char> desc_buf(node.size);
-	if (fread(desc_buf.begin(), node.size, 1, fp) != 1) {
-		return NULL;
-	}
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
-	char *p = desc_buf.begin();
 	way_desc_t *desc = new way_desc_t;
 
 	const uint16 version = node.size==0 ? 0 : decode_uint16(p)&0x7FFFu;
diff --git a/src/simutrans/descriptor/reader/xref_reader.cc b/src/simutrans/descriptor/reader/xref_reader.cc
index a8c97b7a2a..672a4c3193 100644
--- a/src/simutrans/descriptor/reader/xref_reader.cc
+++ b/src/simutrans/descriptor/reader/xref_reader.cc
@@ -4,6 +4,7 @@
  */
 
 #include <stdio.h>
+#include <string.h>
 #include "../../simdebug.h"
 #include "../xref_desc.h"
 #include "xref_reader.h"
@@ -13,25 +14,18 @@
 
 obj_desc_t *xref_reader_t::read_node(FILE *fp, obj_node_info_t &node)
 {
-	char buf[4 + 1];
-	if (fread(buf, 1, 5, fp) != 5) {
+	if (node.size < 5) {
+		dbg->error("xref_reader_t::read_node", "node.size %u < 5", node.size);
 		return NULL;
 	}
+	auto p = node_body(fp, node.size, get_type_name());
+	if (!p) return NULL;
 
-	const uint32 name_len = node.size - 4 - 1;
-	char *p = buf;
+	const uint32 name_len = node.size - 5;
 	xref_desc_t* desc = new(name_len) xref_desc_t();
-
 	desc->type = static_cast<obj_type>(decode_uint32(p));
 	desc->fatal = (decode_uint8(p) != 0);
-
-	if (fread(desc->name, 1, name_len, fp) != name_len) {
-		delete desc;
-		return NULL;
-	}
-
-//	PAKSET_INFO("xref_reader_t::read_node()", "%s",desc->get_text() );
-
+	memcpy(desc->name, p.read_bytes(name_len), name_len);
 	return desc;
 }
 
diff --git a/src/simutrans/descriptor/spezial_obj_tpl.h b/src/simutrans/descriptor/spezial_obj_tpl.h
index 27d17bbe59..ee9a1b8376 100644
--- a/src/simutrans/descriptor/spezial_obj_tpl.h
+++ b/src/simutrans/descriptor/spezial_obj_tpl.h
@@ -35,8 +35,12 @@ template<class desc_t> struct special_obj_tpl {
  */
 template<class desc_t> bool register_desc(special_obj_tpl<desc_t> const* so, desc_t const* const desc)
 {
+	const char* const desc_name = desc->get_name();
+	if (!desc_name) {
+		return false;
+	}
 	for (; so->name; ++so) {
-		if (strcmp(so->name, desc->get_name()) == 0) {
+		if (strcmp(so->name, desc_name) == 0) {
 			if (*so->desc != NULL  ) {
 				// these doublettes are harmless, and hence only recored at debug level 3
 //				dbg->doubled( "object", desc->get_name() );
