Dunedan/0ad
1
0
Fork 0
forked from 0ad/0ad
Code Pull Requests Wiki Activity

Refuse to serialize NaN values.

Browse Source 
NaN values could not be serialised safely because of the multiple
possible NaN numbers.
Since NaN values are usually the result of bugs or dangerous code, it
seems simpler to refuse to serialise them.

(D3205 was a safe-serialization alternative, should the need arise).

Fixes #1879

Differential Revision: https://code.wildfiregames.com/D3729
This was SVN commit r25151.
This commit is contained in:
wraitii 2021-03-28 16:48:25 +00:00
parent f6a2d6da63
commit 0e7fafebe1
2 changed files with 24 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
source/simulation2/serialization/BinarySerializer.cpp
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2020 Wildfire Games.
/* Copyright (C) 2021 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -257,12 +257,21 @@ void CBinarySerializerScriptImpl::HandleScriptVal(JS::HandleValue val)
}
else if (protokey == JSProto_Number)
{
// Standard Number object
m_Serializer.NumberU8_Unbounded("type", SCRIPT_TYPE_OBJECT_NUMBER);
// Get primitive value
double d;
if (!JS::ToNumber(rq.cx, val, &d))
throw PSERROR_Serialize_ScriptError("JS::ToNumber failed");
// Refuse to serialize NaN values: their representation can differ, leading to OOS
// and in general this is indicative of an underlying bug rather than desirable behaviour.
if (std::isnan(d))
{
LOGERROR("Cannot serialize NaN values.");
throw PSERROR_Serialize_InvalidScriptValue();
}
// Standard Number object
m_Serializer.NumberU8_Unbounded("type", SCRIPT_TYPE_OBJECT_NUMBER);
m_Serializer.NumberDouble_Unbounded("value", d);
break;
}
@ -373,12 +382,19 @@ void CBinarySerializerScriptImpl::HandleScriptVal(JS::HandleValue val)
}
case JSTYPE_NUMBER:
{
// Refuse to serialize NaN values: their representation can differ, leading to OOS
// and in general this is indicative of an underlying bug rather than desirable behaviour.
if (val == JS::NaNValue())
{
LOGERROR("Cannot serialize NaN values.");
throw PSERROR_Serialize_InvalidScriptValue();
}
// To reduce the size of the serialized data, we handle integers and doubles separately.
// We can't check for val.isInt32 and val.isDouble directly, because integer numbers are not guaranteed
// to be represented as integers. A number like 33 could be stored as integer on the computer of one player
// and as double on the other player's computer. That would cause out of sync errors in multiplayer games because
// their binary representation and thus the hash would be different.
double d;
d = val.toNumber();
i32 integer;

5
source/simulation2/tests/test_Serializer.h
View File

@ -770,7 +770,10 @@ public:
void test_script_nonfinite()
{
helper_script_roundtrip("nonfinite", "[0, Infinity, -Infinity, NaN, -1/Infinity]", "[0, Infinity, -Infinity, NaN, -0]");
helper_script_roundtrip("nonfinite", "[0, Infinity, -Infinity, -1/Infinity]", "[0, Infinity, -Infinity, -0]");
TestLogger logger;
TS_ASSERT_THROWS(helper_script_roundtrip("nan", "[NaN]", "[NaN]"), const PSERROR_Serialize_InvalidScriptValue&);
}
void test_script_property_order()