Make the two hot decoders as careful as the six cold ones

Every colder message here checks get_available_bytes() before it reads,
and has a test named for it. Snapshots and events -- twenty and sixty
times a second, the two that actually matter -- did neither.

A snapshot's counts are a u8 and two u16s read straight off the wire and
then looped without a bounds check. A garbage enemy count is up to 65535
dictionaries built out of an empty buffer, and a garbage tick is fed
directly to the client's clock estimate and its input-lead resync. The
channel is unreliable and ENet's checksum is off, so a short snapshot is
a network event, not only an attack. It now answers valid:false and the
client drops it -- one dropped snapshot costs 50ms of interpolation.

Events were worse in kind. The decoder read a type byte and matched on
it, so a type it did not recognise consumed one byte and carried on,
reading every field after it at the wrong offset: one stray byte became a
stream of bullets spawned at invented positions, which the client then
applied. Lengths are now declared up front and an unknown type stops the
stream. The prefix is kept, because the channel is reliable and ordered
and throwing the packet away would strand every despawn in it.

Both size tables are pinned to their encoders by tests that encode one of
each record and compare, so they cannot drift -- and a new SimEvent.Type
with no declared length fails a test rather than shipping.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/net/client_runtime.gd

@@ -450,6 +450,12 @@ func on_enter_instance(id: int, kind: int, server_tick: int, boss_id: String,
 
 func on_snapshot(data: PackedByteArray) -> void:
 	var snap := NetCodec.decode_snapshot(data)
+	if not snap["valid"]:
+		# Unreliable channel, no checksum: a truncated snapshot is a network
+		# event, not only an attack. Dropping one costs 50ms of interpolation;
+		# applying half of one moves the clock estimate and the input lead on
+		# numbers read out of the end of a buffer.
+		return
 	if not snap_curr.is_empty() and int(snap["tick"]) <= int(snap_curr["tick"]):
 		return  # stale or duplicate; unreliable channel, newest wins
 	cleared_countdown = int(snap["cleared_countdown"])

src/net/net_codec.gd

@@ -140,18 +140,41 @@ static func encode_snapshot(world: SimWorld,
 	return b.data_array
 
 
+# Fixed record sizes, mirroring what encode_snapshot writes. Pinned against the
+# encoder by test_net_codec so the two cannot drift.
+const SNAPSHOT_HEADER := 5   ## u32 tick + u8 countdown
+const SNAPSHOT_PLAYER := 33
+const SNAPSHOT_ENEMY := 16
+const SNAPSHOT_BOSS := 17
+const SNAPSHOT_LOOT := 13
+
+
+## Decode one snapshot. The returned dictionary carries "valid": a short or
+## corrupt packet answers false and the caller drops it.
+##
+## Snapshots ride an unreliable channel with no checksum, so a truncated one is
+## a network event rather than only an attack -- and this decoder used to read
+## straight through the end of the buffer. A garbage enemy count is up to 65535
+## dictionaries built out of nothing, twenty times a second, and a garbage tick
+## poisons the client's clock estimate and its input lead. Every colder message
+## here already checks; the two hottest did not.
 static func decode_snapshot(data: PackedByteArray) -> Dictionary:
 	var b := StreamPeerBuffer.new()
 	b.big_endian = false
 	b.data_array = data
 	var snap := {
-		"tick": b.get_u32(),
-		"cleared_countdown": b.get_u8(),
+		"tick": 0, "cleared_countdown": Protocol.COUNTDOWN_NONE,
 		"players": [], "enemies": [], "boss": null,
-		"loot": [], "inventory": [],
+		"loot": [], "inventory": [], "valid": false,
 	}
+	if b.get_available_bytes() < SNAPSHOT_HEADER + 1:
+		return snap
+	snap["tick"] = b.get_u32()
+	snap["cleared_countdown"] = b.get_u8()
 
 	var pcount := b.get_u8()
+	if b.get_available_bytes() < pcount * SNAPSHOT_PLAYER:
+		return snap
 	for _i in pcount:
 		snap["players"].append({
 			"peer": b.get_u32(),
@@ -167,7 +190,11 @@ static func decode_snapshot(data: PackedByteArray) -> Dictionary:
 			"total_xp": b.get_u32(),
 		})
 
+	if b.get_available_bytes() < 2:
+		return snap
 	var ecount := b.get_u16()
+	if b.get_available_bytes() < ecount * SNAPSHOT_ENEMY:
+		return snap
 	for _i in ecount:
 		snap["enemies"].append({
 			"id": b.get_u32(),
@@ -177,7 +204,12 @@ static func decode_snapshot(data: PackedByteArray) -> Dictionary:
 			"visual": b.get_u8(),
 		})
 
-	if b.get_u8() == 1:
+	if b.get_available_bytes() < 1:
+		return snap
+	var has_boss := b.get_u8() == 1
+	if has_boss and b.get_available_bytes() < SNAPSHOT_BOSS:
+		return snap
+	if has_boss:
 		snap["boss"] = {
 			"id": b.get_u32(),
 			"pos": Vector2(b.get_float(), b.get_float()),
@@ -185,18 +217,23 @@ static func decode_snapshot(data: PackedByteArray) -> Dictionary:
 			"phase": b.get_u8(),
 		}
 
+	if b.get_available_bytes() < SimConfig.INVENTORY_SLOTS + 2:
+		return snap
 	var inventory: Array[int] = []
 	for _i in SimConfig.INVENTORY_SLOTS:
 		inventory.append(b.get_u8())
 	snap["inventory"] = inventory
 
 	var lcount := b.get_u16()
+	if b.get_available_bytes() < lcount * SNAPSHOT_LOOT:
+		return snap
 	for _i in lcount:
 		snap["loot"].append({
 			"id": b.get_u32(),
 			"pos": Vector2(b.get_float(), b.get_float()),
 			"item": b.get_u8(),
 		})
+	snap["valid"] = true
 	return snap
 
 
@@ -279,16 +316,53 @@ static func encode_events(server_tick: int, events: Array[Dictionary]) -> Packed
 	return b.data_array
 
 
+## Bytes an event of [param t] writes after its type byte, or -1 for a type this
+## build cannot parse.
+##
+## The decoder needs this BEFORE the match, because an event whose length it
+## does not know is not a single bad record: every field after it is read at the
+## wrong offset, so one stray byte becomes a stream of invented bullets at
+## invented positions. Pinned against encode_events by test_net_codec.
+static func event_payload_size(t: int) -> int:
+	match t:
+		SimEvent.Type.BULLET_SPAWN: return 36
+		SimEvent.Type.BULLET_DESPAWN: return 4
+		SimEvent.Type.PLAYER_HIT: return 8
+		SimEvent.Type.PLAYER_DIED, SimEvent.Type.ESCAPE_STARTED, \
+		SimEvent.Type.ESCAPE_CANCELLED, SimEvent.Type.PLAYER_FIRED: return 4
+		SimEvent.Type.ITEM_PICKED_UP, SimEvent.Type.ITEM_USED, \
+		SimEvent.Type.ITEM_DROPPED: return 5
+		SimEvent.Type.PLAYER_RESPAWNED: return 12
+		SimEvent.Type.ENEMY_HIT: return 10
+		SimEvent.Type.ENEMY_DIED: return 4
+		SimEvent.Type.BOSS_PHASE: return 1
+		SimEvent.Type.TELEGRAPH: return 14
+		SimEvent.Type.BOSS_DIED: return 0
+	return -1
+
+
 ## Returns { "tick": int, "events": Array[Dictionary] }.
+##
+## Decoding stops at the first record that does not fit or that names a type
+## this build has no length for. The stream is reliable and ordered, so the
+## prefix decoded before that point is sound and is kept -- unlike a snapshot,
+## dropping the whole packet would strand every bullet it was about to despawn.
 static func decode_events(data: PackedByteArray) -> Dictionary:
 	var out: Array[Dictionary] = []
 	var b := StreamPeerBuffer.new()
 	b.big_endian = false
 	b.data_array = data
+	if b.get_available_bytes() < 6:
+		return {"tick": 0, "events": out}
 	var server_tick := b.get_u32()
 	var count := b.get_u16()
 	for _i in count:
+		if b.get_available_bytes() < 1:
+			break
 		var t := b.get_u8()
+		var payload := event_payload_size(t)
+		if payload < 0 or b.get_available_bytes() < payload:
+			break
 		var ev := {"t": t}
 		match t:
 			SimEvent.Type.BULLET_SPAWN:

tests/unit/test_net_codec.gd

@@ -511,3 +511,143 @@ func test_events_after_a_telegraph_still_decode() -> void:
 	assert_eq(int(out[1]["id"]), 77)
 	assert_eq(int(out[1]["hp"]), 900)
 	assert_eq(int(out[2]["phase"]), 2)
+
+
+# --- Truncation on the two hot streams --------------------------------------
+# Every cold message here already refuses to read past the end of its buffer.
+# Snapshots and events -- the two that arrive twenty and sixty times a second --
+# did not, and the snapshot channel is unreliable and unchecksummed, so a short
+# packet is a network event rather than only an attack.
+
+func _populated_world() -> SimWorld:
+	var w := SimWorld.new(1)
+	w.add_player(1, "a")
+	w.add_player(2, "b")
+	w.spawn_enemy(Content.turret(), Vector2(40.0, 0.0))
+	w.spawn_boss(Content.boss(&"warden"))
+	w.spawn_loot(Items.HEALTH_POTION, Vector2(10.0, 10.0))
+	return w
+
+
+func test_the_snapshot_record_sizes_match_what_the_encoder_writes() -> void:
+	# The guards are arithmetic on these constants, so if they drift from the
+	# encoder the decoder starts refusing good packets or accepting short ones.
+	var empty := SimWorld.new(1)
+	var base := NetCodec.encode_snapshot(empty).size()
+	assert_eq(base, NetCodec.SNAPSHOT_HEADER + 1 + 2 + 1 + SimConfig.INVENTORY_SLOTS + 2,
+		"an empty world is header + three counts + the boss flag + the bag")
+
+	var one_player := SimWorld.new(1)
+	one_player.add_player(1, "a")
+	assert_eq(NetCodec.encode_snapshot(one_player).size() - base,
+		NetCodec.SNAPSHOT_PLAYER)
+
+	var one_enemy := SimWorld.new(1)
+	one_enemy.spawn_enemy(Content.turret(), Vector2.ZERO)
+	assert_eq(NetCodec.encode_snapshot(one_enemy).size() - base,
+		NetCodec.SNAPSHOT_ENEMY)
+
+	var one_boss := SimWorld.new(1)
+	one_boss.spawn_boss(Content.boss(&"warden"))
+	assert_eq(NetCodec.encode_snapshot(one_boss).size() - base,
+		NetCodec.SNAPSHOT_BOSS)
+
+	var one_item := SimWorld.new(1)
+	one_item.spawn_loot(Items.HEALTH_POTION, Vector2.ZERO)
+	assert_eq(NetCodec.encode_snapshot(one_item).size() - base,
+		NetCodec.SNAPSHOT_LOOT)
+
+
+func test_a_truncated_snapshot_is_refused_rather_than_read_past_the_end() -> void:
+	var full := NetCodec.encode_snapshot(_populated_world())
+	assert_true(NetCodec.decode_snapshot(full)["valid"], "the whole packet is fine")
+	# Every prefix short of the whole thing must be refused, not half-applied.
+	for cut in range(0, full.size()):
+		var snap := NetCodec.decode_snapshot(full.slice(0, cut))
+		assert_false(snap["valid"], "a %d-byte prefix must not decode" % cut)
+
+
+func test_a_snapshot_claiming_more_actors_than_it_carries_is_refused() -> void:
+	var data := NetCodec.encode_snapshot(_populated_world())
+	# Byte 5 is the player count; claim 255 of them.
+	data[NetCodec.SNAPSHOT_HEADER] = 255
+	var snap := NetCodec.decode_snapshot(data)
+	assert_false(snap["valid"],
+		"a hostile count is 255 records read out of a buffer that has none")
+	assert_eq((snap["players"] as Array).size(), 0)
+
+
+func test_an_invalid_snapshot_carries_a_usable_empty_shape() -> void:
+	# The caller drops it, but it must not be a shape that errors on the way out.
+	var snap := NetCodec.decode_snapshot(PackedByteArray())
+	assert_false(snap["valid"])
+	assert_eq(snap["tick"], 0)
+	assert_eq((snap["players"] as Array).size(), 0)
+	assert_null(snap["boss"])
+
+
+func test_every_event_type_declares_the_length_the_encoder_writes() -> void:
+	# The decoder needs a length BEFORE it parses, because an event of unknown
+	# size does not spoil one record -- it unaligns every byte after it.
+	var samples := {
+		SimEvent.Type.BULLET_SPAWN: {"uid": 1, "pos": Vector2.ONE, "vel": Vector2.ONE,
+			"r": 4.0, "life": 60, "kind": 0, "team": 0, "accel": 0.0, "turn": 0.0},
+		SimEvent.Type.BULLET_DESPAWN: {"uid": 1},
+		SimEvent.Type.PLAYER_HIT: {"peer": 1, "dmg": 5, "hp": 10},
+		SimEvent.Type.PLAYER_DIED: {"peer": 1},
+		SimEvent.Type.ESCAPE_STARTED: {"peer": 1},
+		SimEvent.Type.ESCAPE_CANCELLED: {"peer": 1},
+		SimEvent.Type.PLAYER_FIRED: {"peer": 1},
+		SimEvent.Type.PLAYER_RESPAWNED: {"peer": 1, "pos": Vector2.ONE},
+		SimEvent.Type.ITEM_PICKED_UP: {"peer": 1, "item": Items.HEALTH_POTION},
+		SimEvent.Type.ITEM_USED: {"peer": 1, "item": Items.HEALTH_POTION},
+		SimEvent.Type.ITEM_DROPPED: {"peer": 1, "item": Items.HEALTH_POTION},
+		SimEvent.Type.ENEMY_HIT: {"id": 1, "dmg": 5, "hp": 10},
+		SimEvent.Type.ENEMY_DIED: {"id": 1},
+		SimEvent.Type.BOSS_PHASE: {"phase": 1},
+		SimEvent.Type.BOSS_DIED: {},
+		SimEvent.Type.TELEGRAPH: {"pos": Vector2.ONE, "r": 8.0, "ticks": 30},
+	}
+	var header := 6   # u32 tick + u16 count
+	for t: int in SimEvent.Type.values():
+		if NetCodec.SERVER_ONLY.has(t):
+			assert_eq(NetCodec.event_payload_size(t), -1,
+				"a server-only event is never on the wire, so it has no length")
+			continue
+		assert_true(samples.has(t),
+			"SimEvent.Type %d is new: give it a sample and a payload size" % t)
+		var ev: Dictionary = (samples[t] as Dictionary).duplicate()
+		ev["t"] = t
+		var wrote := NetCodec.encode_events(0, [ev] as Array[Dictionary]).size()
+		assert_eq(wrote - header - 1, NetCodec.event_payload_size(t),
+			"declared length for SimEvent.Type %d does not match the encoder" % t)
+
+
+func test_a_truncated_event_packet_stops_instead_of_inventing_bullets() -> void:
+	var evs: Array[Dictionary] = [
+		{"t": SimEvent.Type.BULLET_SPAWN, "uid": 7, "pos": Vector2(1.0, 2.0),
+			"vel": Vector2(3.0, 4.0), "r": 4.0, "life": 60, "kind": 0, "team": 1,
+			"accel": 0.0, "turn": 0.0},
+		{"t": SimEvent.Type.ENEMY_DIED, "id": 9},
+	]
+	var full := NetCodec.encode_events(11, evs)
+	assert_eq((NetCodec.decode_events(full)["events"] as Array).size(), 2)
+	# Lop off the last event's payload: the first must still arrive, the second
+	# must not be conjured from whatever is left.
+	var short := full.slice(0, full.size() - 2)
+	var got: Array = NetCodec.decode_events(short)["events"]
+	assert_eq(got.size(), 1, "the sound prefix is kept; the torn record is not")
+	assert_eq(int(got[0]["t"]), SimEvent.Type.BULLET_SPAWN)
+
+
+func test_an_unknown_event_type_stops_the_stream_rather_than_desyncing_it() -> void:
+	var evs: Array[Dictionary] = [{"t": SimEvent.Type.ENEMY_DIED, "id": 9}]
+	var data := NetCodec.encode_events(3, evs)
+	# Claim there are two events and give the second an unparseable type.
+	data[4] = 2
+	data.append(200)
+	data.append_array([1, 2, 3, 4, 5, 6, 7, 8])
+	var got: Array = NetCodec.decode_events(data)["events"]
+	assert_eq(got.size(), 1,
+		"an event whose length is unknown unaligns everything after it, " +
+		"so the only safe answer is to stop")