Remove some remotely-triggerable fatal exceptions

By design, valptridx will throw an exception on invalid input.  This is
better than silently permitting invalid input to corrupt program state.
Past releases blindly trusted that multiplayer peers would not send
invalid input.  Conversion to the valptridx design eliminated the
undefined behavior when peers send invalid input, but still allowed
multiplayer peers to remotely crash the game by sending invalid inputs.
Add a mechanism to trap invalid inputs and gracefully ignore those
messages.  This may cause game consistency issues, but will not allow
data corruption.

common/include/fwd-valptridx.h

@@ -47,6 +47,8 @@ class valptridx :
 	class ic;	/* allow_invalid + const_policy */
 	class vm;	/* require_valid + mutable_policy */
 	class im;	/* allow_invalid + mutable_policy */
+	template <typename>
+		class guarded;
 public:
 	class array_managed_type;
 
@@ -75,6 +77,7 @@ protected:
 	template <typename policy>
 		class basic_ptridx;
 	class allow_end_construction;
+	class assume_nothrow_index;
 
 	static constexpr const array_managed_type &get_array(const_pointer_type p)
 	{

common/include/valptridx.h

@@ -154,6 +154,10 @@ class valptridx<managed_type>::partial_policy::require_valid
 public:
 	static constexpr tt::false_type allow_nullptr{};
 	static constexpr tt::false_type check_allowed_invalid_index(index_type) { return {}; }
+	static constexpr bool check_nothrow_index(index_type i)
+	{
+		return std::less<std::size_t>()(i, array_size);
+	}
 };
 
 template <typename managed_type>
@@ -165,6 +169,10 @@ public:
 	{
 		return i == static_cast<index_type>(~0);
 	}
+	static constexpr bool check_nothrow_index(index_type i)
+	{
+		return check_allowed_invalid_index(i) || require_valid::check_nothrow_index(i);
+	}
 };
 
 template <typename managed_type>
@@ -275,6 +283,10 @@ protected:
 		m_idx(check_index_range<std::less_equal>(DXX_VALPTRIDX_REPORT_STANDARD_LEADER_COMMA_R_PASS_VARS i, &a))
 	{
 	}
+	basic_idx(index_type i, array_managed_type &, const assume_nothrow_index *) :
+		m_idx(i)
+	{
+	}
 public:
 	template <integral_type v>
 		basic_idx(const magic_constant<v> &) :
@@ -385,6 +397,10 @@ public:
 		m_ptr(&a[check_index_range<std::less_equal>(DXX_VALPTRIDX_REPORT_STANDARD_LEADER_COMMA_R_PASS_VARS i, &a)])
 	{
 	}
+	basic_ptr(index_type i, array_managed_type &a, const assume_nothrow_index *) :
+		m_ptr(&a[i])
+	{
+	}
 	basic_ptr(pointer_type p) = delete;
 	basic_ptr(DXX_VALPTRIDX_REPORT_STANDARD_LEADER_COMMA_R_DEFN_VARS pointer_type p, array_managed_type &a) :
 		/* No array consistency check here, since some code incorrectly
@@ -551,6 +567,11 @@ public:
 		vidx_type(DXX_VALPTRIDX_REPORT_STANDARD_LEADER_COMMA_R_PASS_VARS i, a, e)
 	{
 	}
+	basic_ptridx(index_type i, array_managed_type &a, const assume_nothrow_index *e) :
+		vptr_type(i, a, e),
+		vidx_type(i, a, e)
+	{
+	}
 	basic_ptridx(DXX_VALPTRIDX_REPORT_STANDARD_LEADER_COMMA_R_DEFN_VARS pointer_type p, array_managed_type &a) :
 		/* Null pointer is never allowed when an index must be computed.
 		 * Check for null, then use the reference constructor for
@@ -592,6 +613,107 @@ protected:
 	}
 };
 
+template <typename managed_type>
+template <typename guarded_type>
+class valptridx<managed_type>::guarded
+{
+	static_assert(std::is_trivially_destructible<guarded_type>::value, "non-trivial destructor found for guarded_type");
+	enum state : uint8_t
+	{
+		/* empty - the untrusted input was invalid, so no guarded_type
+		 * exists
+		 */
+		empty,
+		/* initialized - the untrusted input was valid, so a
+		 * guarded_type type exists, but the calling code has not yet
+		 * tested the state of this guarded<P>
+		 */
+		initialized,
+		/* checked - the untrusted input was valid, and the calling code
+		 * has called operator bool()
+		 */
+		checked,
+	};
+	union {
+		state m_dummy;
+		guarded_type m_value;
+	};
+	mutable state m_state;
+public:
+	guarded(std::nullptr_t) :
+		m_dummy(), m_state(empty)
+	{
+	}
+	guarded(guarded_type &&v) :
+		m_value(std::move(v)), m_state(initialized)
+	{
+	}
+	__attribute_warn_unused_result
+	explicit operator bool() const
+	{
+		/*
+		 * If no contained guarded_type exists, return false.
+		 * Otherwise, record that the result has been tested and then
+		 * return true.  operator*() uses m_state to enforce that the
+		 * result is tested.
+		 */
+		if (m_state == empty)
+			return false;
+		m_state = checked;
+		return true;
+	}
+	__attribute_warn_unused_result
+	guarded_type operator*() const &
+	{
+		/*
+		 * Correct code will always execute as if this method was just
+		 * the return statement, with none of the sanity checks.  The
+		 * checks are present to catch misuse of this type, preferably
+		 * at compile-time, but at least at runtime.
+		 */
+#define DXX_VALPTRIDX_GUARDED_OBJECT_NO	"access to guarded object that does not exist"
+#define DXX_VALPTRIDX_GUARDED_OBJECT_MAYBE	"access to guarded object that may not exist"
+#ifdef DXX_CONSTANT_TRUE
+		/* If the contained object might not exist: */
+		if (!DXX_CONSTANT_TRUE(m_state == checked))
+		{
+			/*
+			 * Always fail.  Choose an error message and function name
+			 * based on whether the contained type provably does not
+			 * exist.  It provably does not exist if this call is on a
+			 * path where operator bool() returned false.  It
+			 * conditionally might not exist if this call is on a path
+			 * where operator bool() has not been called.
+			 */
+			if (DXX_CONSTANT_TRUE(m_state == empty))
+				DXX_ALWAYS_ERROR_FUNCTION(guarded_accessed_empty, DXX_VALPTRIDX_GUARDED_OBJECT_NO);
+			else
+				DXX_ALWAYS_ERROR_FUNCTION(guarded_accessed_unchecked, DXX_VALPTRIDX_GUARDED_OBJECT_MAYBE);
+		}
+#else
+		/*
+		 * If the compiler does not offer constant truth analysis
+		 * (perhaps because of insufficient optimization), then emit a
+		 * runtime check for whether the guarded_type exists.
+		 *
+		 * This test can throw even if the contained object is valid, if
+		 * the caller did not first validate that the contained object
+		 * is valid.  This restriction is necessary since inputs are
+		 * usually valid even when untested, so throwing only on state
+		 * `empty` would allow incorrect usage to persist in the code
+		 * until someone happened to receive an invalid input from an
+		 * untrusted source.
+		 */
+		if (m_state != checked)
+			throw std::logic_error(m_state == empty ? DXX_VALPTRIDX_GUARDED_OBJECT_NO : DXX_VALPTRIDX_GUARDED_OBJECT_MAYBE);
+#endif
+#undef DXX_VALPTRIDX_GUARDED_OBJECT_MAYBE
+#undef DXX_VALPTRIDX_GUARDED_OBJECT_NO
+		return m_value;
+	}
+	guarded_type operator*() const && = delete;
+};
+
 template <typename managed_type>
 class valptridx<managed_type>::array_managed_type :
 	public detail::valptridx_array_type_count,
@@ -633,6 +755,16 @@ public:
 	basic_ival_global_factory(const basic_ival_global_factory &) = delete;
 	basic_ival_global_factory &operator=(const basic_ival_global_factory &) = delete;
 	__attribute_warn_unused_result
+	guarded<P> check_untrusted(index_type i) const
+	{
+		if (P::check_nothrow_index(i))
+			return P(i, get_array(), static_cast<const assume_nothrow_index *>(nullptr));
+		else
+			return nullptr;
+	}
+	template <typename T>
+		guarded<P> check_untrusted(T &&) const = delete;
+	__attribute_warn_unused_result
 	P operator()(typename P::index_type i DXX_VALPTRIDX_REPORT_STANDARD_LEADER_COMMA_L_DECL_VARS) const
 	{
 		return P(DXX_VALPTRIDX_REPORT_STANDARD_LEADER_COMMA_R_PASS_VARS i, get_array());

similar/main/gamesave.cpp

@@ -1107,10 +1107,10 @@ static int load_game_data(PHYSFS_File *LoadFile)
 				{	//light triggers don't require walls
 					const auto side_num = tr.side[l];
 					auto wall_num = vsegptr(seg_num)->sides[side_num].wall_num;
-					try {
-						vwallptr(wall_num)->controlling_trigger = t;
-					} catch (const valptridx<wall>::index_range_exception &e) {
-						con_puts(CON_URGENT, e.what());
+					if (const auto &&uwall = vwallptr.check_untrusted(wall_num))
+						(*uwall)->controlling_trigger = t;
+					else
+					{
 						LevelError("trigger %u link %u type %u references segment %hu, side %u which is an invalid wall; ignoring.", static_cast<trgnum_t>(t), l, tr.type, seg_num, side_num);
 					}
 				}

similar/main/multi.cpp

@@ -1769,9 +1769,12 @@ static void multi_do_position(const playernum_t pnum, const ubyte *buf)
 
 static void multi_do_reappear(const playernum_t pnum, const ubyte *buf)
 {
-	const objnum_t objnum = GET_INTEL_SHORT(buf + 2);
+	const objnum_t objnum = GET_INTEL_SHORT(&buf[2]);
 
-	auto &obj = *vcobjptr(objnum);
+	const auto &&uobj = vcobjptr.check_untrusted(objnum);
+	if (!uobj)
+		return;
+	auto &obj = **uobj;
 	if (obj.type != OBJ_PLAYER && obj.type != OBJ_GHOST)
 	{
 		con_printf(CON_URGENT, "%s:%u: BUG: object %hu has type %u, expected %u or %u", __FILE__, __LINE__, objnum, obj.type, OBJ_PLAYER, OBJ_GHOST);
@@ -2165,7 +2168,10 @@ static void multi_do_door_open(const ubyte *buf)
 		return;
 	}
 
-	const auto &&seg = vsegptridx(segnum);
+	const auto &&useg = vsegptridx.check_untrusted(segnum);
+	if (!useg)
+		return;
+	const auto &&seg = *useg;
 
 	if (seg->sides[side].wall_num == wall_none) {  //Opening door on illegal wall
 		Int3();
@@ -2214,7 +2220,10 @@ static void multi_do_controlcen_fire(const ubyte *buf)
 	gun_num = buf[count];                       count += 1;
 	objnum = GET_INTEL_SHORT(buf + count);      count += 2;
 
-	auto objp = vobjptridx(objnum);
+	const auto &&uobj = vobjptridx.check_untrusted(objnum);
+	if (!uobj)
+		return;
+	const auto &&objp = *uobj;
 	Laser_create_new_easy(to_target, objp->ctype.reactor_info.gun_pos[gun_num], objp, weapon_id_type::CONTROLCEN_WEAPON_NUM, 1);
 }
 
@@ -2229,7 +2238,10 @@ static void multi_do_create_powerup(const playernum_t pnum, const ubyte *buf)
 
 	count++;
 	powerup_type = buf[count++];
-	const auto &&segnum = vsegptridx(GET_INTEL_SHORT(&buf[count]));
+	const auto &&useg = vsegptridx.check_untrusted(GET_INTEL_SHORT(&buf[count]));
+	if (!useg)
+		return;
+	const auto &&segnum = *useg;
 	count += 2;
 	objnum_t objnum = GET_INTEL_SHORT(buf + count); count += 2;
 	memcpy(&new_pos, buf+count, sizeof(vms_vector)); count+=sizeof(vms_vector);
@@ -2324,7 +2336,9 @@ static void multi_do_effect_blowup(const playernum_t pnum, const ubyte *buf)
 
 	multi_do_protocol_frame(1, 0); // force packets to be sent, ensuring this packet will be attached to following MULTI_TRIGGER
 
-	segnum_t segnum = GET_INTEL_SHORT(buf + 2); 
+	const auto &&useg = vsegptridx.check_untrusted(GET_INTEL_SHORT(&buf[2]));
+	if (!useg)
+		return;
 	side = buf[4];
 	hitpnt.x = GET_INTEL_INT(buf + 5);
 	hitpnt.y = GET_INTEL_INT(buf + 9);
@@ -2337,7 +2351,7 @@ static void multi_do_effect_blowup(const playernum_t pnum, const ubyte *buf)
 	laser.parent_type = OBJ_PLAYER;
 	laser.parent_num = pnum;
 
-	check_effect_blowup(vsegptridx(segnum), side, hitpnt, laser, 0, 1);
+	check_effect_blowup(*useg, side, hitpnt, laser, 0, 1);
 }
 
 static void multi_do_drop_marker (const playernum_t pnum, const ubyte *buf)
@@ -3949,7 +3963,10 @@ static void multi_do_light (const ubyte *buf)
 	const auto sides = buf[3];
 
 	const segnum_t seg = GET_INTEL_SHORT(&buf[1]);
-	const auto &&segp = vsegptridx(seg);
+	const auto &&usegp = vsegptridx.check_untrusted(seg);
+	if (!usegp)
+		return;
+	const auto &&segp = *usegp;
 	auto &side_array = segp->sides;
 	for (i=0;i<6;i++)
 	{

similar/main/switch.cpp

@@ -174,11 +174,11 @@ static int do_change_walls(const trigger &t, const uint8_t new_wall_type)
 			}
 
 			wall *w0p;
-			try {
 				const auto w0num = segp->sides[side].wall_num;
-				w0p = vwallptr(w0num);
-			} catch (const valptridx<wall>::index_range_exception &e) {
-				con_puts(CON_URGENT, e.what());
+			if (const auto &&uw0p = vwallptr.check_untrusted(w0num))
+				w0p = *uw0p;
+			else
+			{
 				LevelError("trigger %p link %u tried to open segment %hu, side %u which is an invalid wall; ignoring.", addressof(t), i, static_cast<segnum_t>(segp), side);
 				continue;
 			}