From 5ca2f59619cbb4e3e46d502e58ba029066c063e9 Mon Sep 17 00:00:00 2001 From: Kp Date: Mon, 15 May 2023 23:52:18 +0000 Subject: [PATCH] Validate last_hitobj on game load Testing for `object_none` is insufficient. Games saved before the fix for 14cdf1b3521ff82701c58c04e47a6c1deefe8e43 may have a `last_hitobj` of `0x1ff`, which triggers an exception when constructing `icobjidx_t`. Treat any invalid object index as `object_none`. Reported-by: Quix0r --- similar/main/multi.cpp | 7 ++++--- similar/main/state.cpp | 14 +++++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/similar/main/multi.cpp b/similar/main/multi.cpp index 66cf32842..861230712 100644 --- a/similar/main/multi.cpp +++ b/similar/main/multi.cpp @@ -6158,10 +6158,11 @@ void multi_object_rw_to_object(const object_rw *const obj_rw, object &obj) obj.ctype.laser_info.parent_num = INTEL_SHORT(obj_rw->ctype.laser_info.parent_num); obj.ctype.laser_info.parent_signature = object_signature_t{static_cast(INTEL_INT(obj_rw->ctype.laser_info.parent_signature))}; obj.ctype.laser_info.creation_time = INTEL_INT(obj_rw->ctype.laser_info.creation_time); - { - const auto last_hitobj = INTEL_SHORT(obj_rw->ctype.laser_info.last_hitobj); + /* `last_hitobj` is untrusted network data, so it must be checked before use. */ + if (const auto last_hitobj = INTEL_SHORT(obj_rw->ctype.laser_info.last_hitobj); vcobjidx_t::check_nothrow_index(last_hitobj)) obj.ctype.laser_info.reset_hitobj(last_hitobj); - } + else + obj.ctype.laser_info.clear_hitobj(); obj.ctype.laser_info.track_goal = INTEL_SHORT(obj_rw->ctype.laser_info.track_goal); obj.ctype.laser_info.multiplier = INTEL_INT(obj_rw->ctype.laser_info.multiplier); #if defined(DXX_BUILD_DESCENT_II) diff --git a/similar/main/state.cpp b/similar/main/state.cpp index b4e6e2281..bd14874d6 100644 --- a/similar/main/state.cpp +++ b/similar/main/state.cpp @@ -601,7 +601,19 @@ static void state_object_rw_to_object(const object_rw *const obj_rw, object &obj obj.ctype.laser_info.parent_num = obj_rw->ctype.laser_info.parent_num; obj.ctype.laser_info.parent_signature = object_signature_t{static_cast(obj_rw->ctype.laser_info.parent_signature)}; obj.ctype.laser_info.creation_time = obj_rw->ctype.laser_info.creation_time; - obj.ctype.laser_info.reset_hitobj(obj_rw->ctype.laser_info.last_hitobj); + /* Use vcobjidx_t so that `object_none` fails the test and uses the + * `else` path. `reset_hitobj` can accept `object_none`, but it + * cannot accept invalid indexes, such as the corrupted value + * `0x1ff` produced by certain <0.60 builds, when they ran + * `laser_info.hitobj_list[-1] = 1;` due to using `object_none` + * (`-1` back then) as if it were a valid index. That bug was + * fixed in 14cdf1b3521ff82701c58c04e47a6c1deefe8e43, but some old + * save games were corrupted by it, so trap that error here. + */ + if (const auto last_hitobj = obj_rw->ctype.laser_info.last_hitobj; vcobjidx_t::check_nothrow_index(last_hitobj)) + obj.ctype.laser_info.reset_hitobj(last_hitobj); + else + obj.ctype.laser_info.clear_hitobj(); obj.ctype.laser_info.track_goal = obj_rw->ctype.laser_info.track_goal; obj.ctype.laser_info.multiplier = obj_rw->ctype.laser_info.multiplier; #if defined(DXX_BUILD_DESCENT_II)