Add network serialization float NaN sanitization

Apparently cheat clients have figured out that none of SS14's code does validation against NaN inputs. Uh oh.

IRobustSerializer can now be configured to remove NaN values when reading. This is intended to be set on the server to completely block the issue.

Added "Unsafe" float types that can be used to bypass the new configurable behavior, in case somebody *really* needs NaNs.

An alternative option was to make a "SafeFloat" type, and only apply the sanitization to that. The problem is that would require updating hundreds if not thousands of messages in SS14, and probably significantly confuse contributors on "when use what." Blocking NaNs by default is likely to cause little issues while ensuring the entire exploit is guaranteed impossible.

RELEASE-NOTES.md

@@ -39,7 +39,10 @@ END TEMPLATE-->
 
 ### New features
 
-*None yet*
+* `IRobustSerializer` can now be configured to remove float NaN values when reading.
+  * This is intended to blanket block cheat clients from sending NaN values in input commands they shouldn't.
+  * To enable, set `IRobustSerializer.FloatFlags` from your content entrypoint.
+  * If you do really want to send NaN values while using the above, you can use the new `UnsafeFloat`, `UnsafeHalf`, and `UnsafeDouble` types to indicate a field that is exempt.
 
 ### Bugfixes
 

Robust.Shared.IntegrationTests/Serialization/NetSerializerDefaultFloatTest.cs

@@ -0,0 +1,203 @@
+using JetBrains.Annotations;
+using NUnit.Framework;
+using Robust.Shared.IoC;
+using Robust.Shared.Maths;
+using Robust.Shared.Serialization;
+using Robust.UnitTesting.Shared;
+
+namespace Robust.Shared.IntegrationTests.Serialization;
+
+[Serializable, NetSerializable]
+[UsedImplicitly(Reason = "Needed so RobustSerializer is guaranteed to pick up on the unsafe types.")]
+internal sealed class MakeTheseSerializable
+{
+    public UnsafeFloat Single;
+    public UnsafeDouble Double;
+    public UnsafeHalf Half;
+    public Half SafeHalf;
+}
+
+/// <summary>
+/// Tests the serialization behavior of float types when <see cref="IRobustSerializer"/> is *not* set to do anything special.
+/// Tests both primitives and Robust's "Unsafe" variants.
+/// </summary>
+[TestFixture, TestOf(typeof(RobustSerializer)), TestOf(typeof(NetUnsafeFloatSerializer))]
+internal sealed class NetSerializerDefaultFloatTest : OurRobustUnitTest
+{
+    private IRobustSerializer _serializer = null!;
+
+    [OneTimeSetUp]
+    public void Setup()
+    {
+        _serializer = IoCManager.Resolve<IRobustSerializer>();
+        _serializer.Initialize();
+    }
+
+    internal static readonly TestCaseData[] PassThroughFloatTests =
+    [
+        new TestCaseData(0.0).Returns(0.0),
+        new TestCaseData(1.0).Returns(1.0),
+        new TestCaseData(double.NaN).Returns(double.NaN),
+        new TestCaseData(double.PositiveInfinity).Returns(double.PositiveInfinity),
+    ];
+
+    [TestCaseSource(nameof(PassThroughFloatTests))]
+    public double TestSingle(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (float)input);
+
+        ms.Position = 0;
+
+        return _serializer.Deserialize<float>(ms);
+    }
+
+    [TestCaseSource(nameof(PassThroughFloatTests))]
+    public double TestUnsafeSingle(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (UnsafeFloat)input);
+
+        ms.Position = 0;
+
+        return _serializer.Deserialize<UnsafeFloat>(ms);
+    }
+
+    [TestCaseSource(nameof(PassThroughFloatTests))]
+    public double TestDouble(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, input);
+
+        ms.Position = 0;
+
+        return _serializer.Deserialize<double>(ms);
+    }
+
+    [TestCaseSource(nameof(PassThroughFloatTests))]
+    public double TestUnsafeDouble(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (UnsafeDouble)input);
+
+        ms.Position = 0;
+
+        return _serializer.Deserialize<UnsafeDouble>(ms);
+    }
+
+    [TestCaseSource(nameof(PassThroughFloatTests))]
+    public double TestHalf(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (Half)input);
+
+        ms.Position = 0;
+
+        return (double)_serializer.Deserialize<Half>(ms);
+    }
+
+    [TestCaseSource(nameof(PassThroughFloatTests))]
+    public double TestUnsafeHalf(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (UnsafeHalf)(Half)input);
+
+        ms.Position = 0;
+
+        return (double)(Half)_serializer.Deserialize<UnsafeHalf>(ms);
+    }
+}
+
+/// <summary>
+/// Tests the serialization behavior of float types when <see cref="IRobustSerializer"/> is set to remove NaNs on read.
+/// Tests both primitives and Robust's "Unsafe" variants.
+/// </summary>
+[TestFixture]
+[TestOf(typeof(RobustSerializer)), TestOf(typeof(NetUnsafeFloatSerializer)), TestOf(typeof(NetSafeFloatSerializer))]
+internal sealed class NetSerializerSafeFloatTest : OurRobustUnitTest
+{
+    private IRobustSerializer _serializer = default!;
+
+    [OneTimeSetUp]
+    public void Setup()
+    {
+        _serializer = IoCManager.Resolve<IRobustSerializer>();
+        _serializer.FloatFlags = SerializerFloatFlags.RemoveReadNan;
+        _serializer.Initialize();
+    }
+
+    internal static readonly TestCaseData[] SafeFloatTests =
+    [
+        new TestCaseData(0.0).Returns(0.0),
+        new TestCaseData(1.0).Returns(1.0),
+        new TestCaseData(double.NaN).Returns(0.0),
+        new TestCaseData(double.PositiveInfinity).Returns(double.PositiveInfinity),
+    ];
+
+    [TestCaseSource(nameof(SafeFloatTests))]
+    public double TestSingle(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (float)input);
+
+        ms.Position = 0;
+
+        return _serializer.Deserialize<float>(ms);
+    }
+
+    [TestCaseSource(typeof(NetSerializerDefaultFloatTest), nameof(NetSerializerDefaultFloatTest.PassThroughFloatTests))]
+    public double TestUnsafeSingle(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (UnsafeFloat)input);
+
+        ms.Position = 0;
+
+        return _serializer.Deserialize<UnsafeFloat>(ms);
+    }
+
+    [TestCaseSource(nameof(SafeFloatTests))]
+    public double TestDouble(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, input);
+
+        ms.Position = 0;
+
+        return _serializer.Deserialize<double>(ms);
+    }
+
+    [TestCaseSource(typeof(NetSerializerDefaultFloatTest), nameof(NetSerializerDefaultFloatTest.PassThroughFloatTests))]
+    public double TestUnsafeDouble(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (UnsafeDouble)input);
+
+        ms.Position = 0;
+
+        return _serializer.Deserialize<UnsafeDouble>(ms);
+    }
+
+
+    [TestCaseSource(nameof(SafeFloatTests))]
+    public double TestHalf(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (Half)input);
+
+        ms.Position = 0;
+
+        return (double)_serializer.Deserialize<Half>(ms);
+    }
+
+    [TestCaseSource(typeof(NetSerializerDefaultFloatTest), nameof(NetSerializerDefaultFloatTest.PassThroughFloatTests))]
+    public double TestUnsafeHalf(double input)
+    {
+        var ms = new MemoryStream();
+        _serializer.Serialize(ms, (UnsafeHalf)(Half)input);
+
+        ms.Position = 0;
+
+        return (double)(Half)_serializer.Deserialize<UnsafeHalf>(ms);
+    }
+}

Robust.Shared.Maths/UnsafeFloat.cs

@@ -0,0 +1,53 @@
+using System;
+
+namespace Robust.Shared.Maths;
+
+/// <summary>
+/// Marker type to indicate floating point values that should preserve NaNs across the network.
+/// </summary>
+/// <remarks>
+/// Robust's network serializer may be configured to flush NaN float values to 0,
+/// to avoid exploits from lacking input validation. Even if this feature is enabled,
+/// NaN values passed in this type are still untouched.
+/// </remarks>
+/// <param name="Value">The actual inner floating point value</param>
+/// <seealso cref="System.Half"/>
+public readonly record struct UnsafeHalf(Half Value)
+{
+    public static implicit operator Half(UnsafeHalf f) => f.Value;
+    public static implicit operator UnsafeHalf(Half f) => new(f);
+}
+
+/// <summary>
+/// Marker type to indicate floating point values that should preserve NaNs across the network.
+/// </summary>
+/// <remarks>
+/// Robust's network serializer may be configured to flush NaN float values to 0,
+/// to avoid exploits from lacking input validation. Even if this feature is enabled,
+/// NaN values passed in this type are still untouched.
+/// </remarks>
+/// <param name="Value">The actual inner floating point value</param>
+/// <seealso cref="System.Single"/>
+public readonly record struct UnsafeFloat(float Value)
+{
+    public static implicit operator float(UnsafeFloat f) => f.Value;
+    public static implicit operator UnsafeFloat(float f) => new(f);
+}
+
+/// <summary>
+/// Marker type to indicate floating point values that should preserve NaNs across the network.
+/// </summary>
+/// <remarks>
+/// Robust's network serializer may be configured to flush NaN float values to 0,
+/// to avoid exploits from lacking input validation. Even if this feature is enabled,
+/// NaN values passed in this type are still untouched.
+/// </remarks>
+/// <param name="Value">The actual inner floating point value</param>
+/// <seealso cref="System.Double"/>
+public readonly record struct UnsafeDouble(double Value)
+{
+    public static implicit operator double(UnsafeDouble f) => f.Value;
+    public static implicit operator UnsafeDouble(double f) => new(f);
+    public static implicit operator UnsafeDouble(float f) => new(f);
+    public static implicit operator UnsafeDouble(UnsafeFloat f) => new(f);
+}

Robust.Shared/Serialization/IRobustSerializer.cs

@@ -2,6 +2,8 @@ using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Threading.Tasks;
+using Robust.Shared.ContentPack;
+using Robust.Shared.Maths;
 using Robust.Shared.Network;
 
 namespace Robust.Shared.Serialization
@@ -9,6 +11,21 @@ namespace Robust.Shared.Serialization
     [NotContentImplementable]
     public interface IRobustSerializer
     {
+        /// <summary>
+        /// Specifies how the serializer should handle read floating point values.
+        /// </summary>
+        /// <remarks>
+        /// Both sides of the network need not have the same float handling flags.
+        /// </remarks>
+        /// <exception cref="InvalidOperationException">
+        /// Thrown if set after the serializer has already been initialized.
+        /// (must be done from <see cref="ModRunLevel.PreInit"/>)
+        /// </exception>
+        SerializerFloatFlags FloatFlags { get; set; }
+
+        /// <exception cref="InvalidOperationException">
+        /// Thrown if called twice.
+        /// </exception>
         void Initialize();
         void Serialize(Stream stream, object toSerialize);
 
@@ -70,4 +87,25 @@ namespace Robust.Shared.Serialization
         long BytesDeserialized { get; }
         long ObjectsDeserialized { get; }
     }
+
+    /// <summary>
+    /// Flags for <see cref="IRobustSerializer"/> float handling.
+    /// </summary>
+    /// <remarks>
+    /// These flags have no effect on values passed in a <see cref="UnsafeFloat"/>, <see cref="UnsafeHalf"/> or
+    /// <see cref="UnsafeDouble"/>.
+    /// </remarks>
+    [Flags]
+    public enum SerializerFloatFlags
+    {
+        /// <summary>
+        /// No special behavior: floating point values are read exactly as sent over the network.
+        /// </summary>
+        None = 0,
+
+        /// <summary>
+        /// Read NaN values will be cleared to zero.
+        /// </summary>
+        RemoveReadNan = 1 << 0,
+    }
 }

Robust.Shared/Serialization/NetSafeFloatSerializer.cs

@@ -0,0 +1,37 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Reflection;
+using NetSerializer;
+
+namespace Robust.Shared.Serialization;
+
+/// <summary>
+/// Replaces NetSerializer's default float handling to read NaN values as 0.
+/// </summary>
+internal sealed class NetSafeFloatSerializer : IStaticTypeSerializer
+{
+    public bool Handles(Type type)
+    {
+        return type == typeof(float) || type == typeof(double) || type == typeof(Half);
+    }
+
+    public IEnumerable<Type> GetSubtypes(Type type)
+    {
+        return [];
+    }
+
+    public MethodInfo GetStaticWriter(Type type)
+    {
+        return typeof(Primitives).GetMethod(nameof(Primitives.WritePrimitive),
+            BindingFlags.Public | BindingFlags.Static,
+            [typeof(Stream), type])!;
+    }
+
+    public MethodInfo GetStaticReader(Type type)
+    {
+        return typeof(SafePrimitives).GetMethod(nameof(SafePrimitives.ReadPrimitive),
+            BindingFlags.Public | BindingFlags.Static,
+            [typeof(Stream), type.MakeByRefType()])!;
+    }
+}

Robust.Shared/Serialization/NetUnsafeFloatSerializer.cs

@@ -0,0 +1,78 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Reflection;
+using JetBrains.Annotations;
+using NetSerializer;
+using Robust.Shared.Maths;
+
+namespace Robust.Shared.Serialization;
+
+/// <summary>
+/// NetSerializer type serializer for <see cref="UnsafeFloat"/>, <see cref="UnsafeHalf"/>, and <see cref="UnsafeFloat"/>.
+/// </summary>
+internal sealed class NetUnsafeFloatSerializer : IStaticTypeSerializer
+{
+    public bool Handles(Type type)
+    {
+        return type == typeof(UnsafeFloat) || type == typeof(UnsafeDouble) || type == typeof(UnsafeHalf);
+    }
+
+    public IEnumerable<Type> GetSubtypes(Type type)
+    {
+        return [];
+    }
+
+    public MethodInfo GetStaticWriter(Type type)
+    {
+        return typeof(NetUnsafeFloatSerializer).GetMethod(nameof(Write),
+            BindingFlags.NonPublic | BindingFlags.Static,
+            [typeof(Stream), type])!;
+    }
+
+    public MethodInfo GetStaticReader(Type type)
+    {
+        return typeof(NetUnsafeFloatSerializer).GetMethod(nameof(Read),
+            BindingFlags.NonPublic | BindingFlags.Static,
+            [typeof(Stream), type.MakeByRefType()])!;
+    }
+
+    [UsedImplicitly]
+    private static void Write(Stream stream, UnsafeFloat value)
+    {
+        Primitives.WritePrimitive(stream, value);
+    }
+
+    [UsedImplicitly]
+    private static void Read(Stream stream, out UnsafeFloat value)
+    {
+        Primitives.ReadPrimitive(stream, out float readValue);
+        value = readValue;
+    }
+
+    [UsedImplicitly]
+    private static void Write(Stream stream, UnsafeDouble value)
+    {
+        Primitives.WritePrimitive(stream, value);
+    }
+
+    [UsedImplicitly]
+    private static void Read(Stream stream, out UnsafeDouble value)
+    {
+        Primitives.ReadPrimitive(stream, out double readValue);
+        value = readValue;
+    }
+
+    [UsedImplicitly]
+    private static void Write(Stream stream, UnsafeHalf value)
+    {
+        Primitives.WritePrimitive(stream, value);
+    }
+
+    [UsedImplicitly]
+    private static void Read(Stream stream, out UnsafeHalf value)
+    {
+        Primitives.ReadPrimitive(stream, out Half readValue);
+        value = readValue;
+    }
+}

Robust.Shared/Serialization/RobustSerializer.cs

@@ -27,6 +27,8 @@ namespace Robust.Shared.Serialization
         private Serializer _serializer = default!;
 
         private HashSet<Type> _serializableTypes = default!;
+        private bool _initialized;
+        private SerializerFloatFlags _floatFlags;
 
         private static Type[] AlwaysNetSerializable => new[]
         {
@@ -56,8 +58,25 @@ namespace Robust.Shared.Serialization
 
         #endregion
 
+        public SerializerFloatFlags FloatFlags
+        {
+            get => _floatFlags;
+            set
+            {
+                if (_initialized)
+                    throw new InvalidOperationException("Already initialized!");
+
+                _floatFlags = value;
+            }
+        }
+
         public void Initialize()
         {
+            if (_initialized)
+                throw new InvalidOperationException("Already initialized!");
+
+            _initialized = true;
+
             var types = _reflectionManager.FindTypesWithAttribute<NetSerializableAttribute>()
                 .OrderBy(x => x.FullName, StringComparer.InvariantCulture)
                 .ToList();
@@ -91,9 +110,21 @@ namespace Robust.Shared.Serialization
                     MappedStringSerializer.TypeSerializer,
                     new NetMathSerializer(),
                     new NetBitArraySerializer(),
-                    new NetFormattedStringSerializer()
+                    new NetFormattedStringSerializer(),
+                    new NetUnsafeFloatSerializer(),
                 }
             };
+
+            if ((_floatFlags & SerializerFloatFlags.RemoveReadNan) != 0)
+            {
+                settings.CustomTypeSerializers =
+                [
+                    ..settings.CustomTypeSerializers,
+                    // This replaces NetSerializer's default serializer.
+                    new NetSafeFloatSerializer()
+                ];
+            }
+
             _serializer = new Serializer(types, settings);
             _serializableTypes = new HashSet<Type>(_serializer.GetTypeMap().Keys);
             LogSzr.Info($"Serializer Types Hash: {_serializer.GetSHA256()}");

Robust.Shared/Serialization/SafePrimitives.cs

@@ -0,0 +1,45 @@
+using System;
+using System.IO;
+using JetBrains.Annotations;
+using NetSerializer;
+
+namespace Robust.Shared.Serialization;
+
+/// <summary>
+/// "Safer" read primitives as an alternative to <see cref="Primitives"/>.
+/// </summary>
+internal static class SafePrimitives
+{
+    /// <summary>
+    /// Read a float value from the stream, flushing NaNs to zero.
+    /// </summary>
+    [UsedImplicitly]
+    public static void ReadPrimitive(Stream stream, out float value)
+    {
+        Primitives.ReadPrimitive(stream, out float readFloat);
+
+        value = float.IsNaN(readFloat) ? 0 : readFloat;
+    }
+
+    /// <summary>
+    /// Read a double value from the stream, flushing NaNs to zero.
+    /// </summary>
+    [UsedImplicitly]
+    public static void ReadPrimitive(Stream stream, out double value)
+    {
+        Primitives.ReadPrimitive(stream, out double readDouble);
+
+        value = double.IsNaN(readDouble) ? 0 : readDouble;
+    }
+
+    /// <summary>
+    /// Read a double value from the stream, flushing NaNs to zero.
+    /// </summary>
+    [UsedImplicitly]
+    public static void ReadPrimitive(Stream stream, out Half value)
+    {
+        Primitives.ReadPrimitive(stream, out Half readDouble);
+
+        value = Half.IsNaN(readDouble) ? Half.Zero : readDouble;
+    }
+}

Robust.Shared/Serialization/TypeSerializers/Implementations/Primitive/UnsafeFloatSerializer.cs

@@ -0,0 +1,103 @@
+using Robust.Shared.IoC;
+using Robust.Shared.Maths;
+using Robust.Shared.Serialization.Manager;
+using Robust.Shared.Serialization.Manager.Attributes;
+using Robust.Shared.Serialization.Markdown;
+using Robust.Shared.Serialization.Markdown.Validation;
+using Robust.Shared.Serialization.Markdown.Value;
+using Robust.Shared.Serialization.TypeSerializers.Interfaces;
+
+namespace Robust.Shared.Serialization.TypeSerializers.Implementations.Primitive;
+
+/// <summary>
+/// Implementation of type serializers for <see cref="UnsafeFloat"/> and <see cref="UnsafeDouble"/>.
+/// </summary>
+/// <remarks>
+/// These don't need to do anything different from <see cref="FloatSerializer"/> and <see cref="DoubleSerializer"/>,
+/// because YAML cannot contain NaNs.
+/// </remarks>
+[TypeSerializer]
+internal sealed class UnsafeFloatSerializer :
+    ITypeSerializer<UnsafeFloat, ValueDataNode>, ITypeCopyCreator<UnsafeFloat>,
+    ITypeSerializer<UnsafeDouble, ValueDataNode>, ITypeCopyCreator<UnsafeDouble>
+{
+    ValidationNode ITypeValidator<UnsafeFloat, ValueDataNode>.Validate(
+        ISerializationManager serializationManager,
+        ValueDataNode node,
+        IDependencyCollection dependencies,
+        ISerializationContext? context)
+    {
+        return serializationManager.ValidateNode<float>(node, context);
+    }
+
+    public UnsafeFloat Read(
+        ISerializationManager serializationManager,
+        ValueDataNode node,
+        IDependencyCollection dependencies,
+        SerializationHookContext hookCtx,
+        ISerializationContext? context = null,
+        ISerializationManager.InstantiationDelegate<UnsafeFloat>? instanceProvider = null)
+    {
+        return serializationManager.Read<float>(node, hookCtx, context);
+    }
+
+    public DataNode Write(
+        ISerializationManager serializationManager,
+        UnsafeFloat value,
+        IDependencyCollection dependencies,
+        bool alwaysWrite = false,
+        ISerializationContext? context = null)
+    {
+        return serializationManager.WriteValue(value.Value, alwaysWrite, context);
+    }
+
+    ValidationNode ITypeValidator<UnsafeDouble, ValueDataNode>.Validate(
+        ISerializationManager serializationManager,
+        ValueDataNode node,
+        IDependencyCollection dependencies,
+        ISerializationContext? context)
+    {
+        return serializationManager.ValidateNode<double>(node, context);
+    }
+
+    public UnsafeDouble Read(
+        ISerializationManager serializationManager,
+        ValueDataNode node,
+        IDependencyCollection dependencies,
+        SerializationHookContext hookCtx,
+        ISerializationContext? context = null,
+        ISerializationManager.InstantiationDelegate<UnsafeDouble>? instanceProvider = null)
+    {
+        return serializationManager.Read<double>(node, hookCtx, context);
+    }
+
+    public DataNode Write(
+        ISerializationManager serializationManager,
+        UnsafeDouble value,
+        IDependencyCollection dependencies,
+        bool alwaysWrite = false,
+        ISerializationContext? context = null)
+    {
+        return serializationManager.WriteValue(value.Value, alwaysWrite, context);
+    }
+
+    public UnsafeFloat CreateCopy(
+        ISerializationManager serializationManager,
+        UnsafeFloat source,
+        IDependencyCollection dependencies,
+        SerializationHookContext hookCtx,
+        ISerializationContext? context = null)
+    {
+        return source;
+    }
+
+    public UnsafeDouble CreateCopy(
+        ISerializationManager serializationManager,
+        UnsafeDouble source,
+        IDependencyCollection dependencies,
+        SerializationHookContext hookCtx,
+        ISerializationContext? context = null)
+    {
+        return source;
+    }
+}