redis: a reply nested more than 512 arrays deep fails the command instead of overflowing the stack

src/clients/ioxide.redis/RespProtocol.cs

@@ -5,6 +5,9 @@ namespace ioxide.redis;
 /// <summary>RESP2 wire format: write a command as an array of bulk strings; parse one reply.</summary>
 internal static class RespProtocol
 {
+    // Real replies nest a few arrays deep; the parser recurses per level, on the reactor's stack.
+    private const int MaxDepth = 512;
+
     /// <summary>
     /// Bytes needed to encode a command as a RESP array of bulk strings, given a pre-framed name
     /// token (<c>$len\r\nNAME\r\n</c>, see <see cref="FrameName"/>) plus the argument list.
@@ -95,7 +98,7 @@ public static bool TryParse(ReadOnlySpan<byte> buffer, out RespValue value, out
     {
         int pos = 0;
         needed = 0;
-        if (TryParseAt(buffer, ref pos, out value, ref needed))
+        if (TryParseAt(buffer, ref pos, out value, ref needed, depth: 0))
         {
             consumed = pos;
             return true;
@@ -104,7 +107,7 @@ public static bool TryParse(ReadOnlySpan<byte> buffer, out RespValue value, out
         return false;
     }
 
-    private static bool TryParseAt(ReadOnlySpan<byte> buffer, ref int pos, out RespValue value, ref int needed)
+    private static bool TryParseAt(ReadOnlySpan<byte> buffer, ref int pos, out RespValue value, ref int needed, int depth)
     {
         value = default;
         if (pos >= buffer.Length)
@@ -180,11 +183,16 @@ private static bool TryParseAt(ReadOnlySpan<byte> buffer, ref int pos, out RespV
                     throw new RedisException($"invalid array length {count}");
                 }
 
+                if (depth >= MaxDepth)
+                {
+                    throw new RedisException($"RESP reply nested more than {MaxDepth} arrays deep");
+                }
+
                 var items = new RespValue[count];
                 int p = afterLine;
                 for (int i = 0; i < count; i++)
                 {
-                    if (!TryParseAt(buffer, ref p, out RespValue item, ref needed))
+                    if (!TryParseAt(buffer, ref p, out RespValue item, ref needed, depth + 1))
                     {
                         return false;
                     }

tests/Ioxide.Tests.Harness/FakeServer.cs

@@ -0,0 +1,112 @@
+using System.Net;
+using System.Net.Sockets;
+
+namespace Ioxide.Tests;
+
+/// <summary>
+/// A loopback server that misbehaves on purpose, for the far end of a protocol client: each
+/// connection it accepts runs a script on a thread of its own, which speaks just enough of the
+/// protocol and then sends the offending bytes or goes silent. Disposing it closes the listener
+/// and every connection it accepted.
+/// </summary>
+public sealed class FakeServer : IDisposable
+{
+    private readonly Socket _listener;
+    private readonly Action<int, Socket> _script;
+    private readonly List<Socket> _accepted = [];
+    private readonly Lock _lock = new();
+    private bool _disposed;
+
+    /// <summary>The port it listens on, on 127.0.0.1.</summary>
+    public int Port { get; }
+
+    /// <param name="script">
+    /// Run once per accepted connection, with its index (0 for the first) and its socket. A script
+    /// that returns leaves the connection open until the server is disposed.
+    /// </param>
+    public FakeServer(Action<int, Socket> script)
+    {
+        _script = script;
+        _listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
+        _listener.Bind(new IPEndPoint(IPAddress.Loopback, 0));
+        _listener.Listen(16);
+        Port = ((IPEndPoint)_listener.LocalEndPoint!).Port;
+
+        new Thread(AcceptLoop) { IsBackground = true, Name = $"fake-server-{Port}" }.Start();
+    }
+
+    /// <summary>Reads exactly <paramref name="count"/> bytes, or throws if the peer closes first.</summary>
+    public static byte[] Receive(Socket socket, int count)
+    {
+        var buffer = new byte[count];
+        for (int filled = 0; filled < count;)
+        {
+            int n = socket.Receive(buffer, filled, count - filled, SocketFlags.None);
+            if (n == 0)
+            {
+                throw new IOException($"the peer closed after {filled} of {count} bytes");
+            }
+
+            filled += n;
+        }
+
+        return buffer;
+    }
+
+    private void AcceptLoop()
+    {
+        for (int index = 0; ; index++)
+        {
+            Socket accepted;
+            try
+            {
+                accepted = _listener.Accept();
+            }
+            catch (Exception)
+            {
+                return;   // disposed
+            }
+
+            lock (_lock)
+            {
+                if (_disposed)
+                {
+                    accepted.Dispose();
+                    return;
+                }
+
+                _accepted.Add(accepted);
+            }
+
+            int which = index;
+            new Thread(() =>
+            {
+                try
+                {
+                    _script(which, accepted);
+                }
+                catch (Exception)
+                {
+                    // The client hung up mid-script, or Dispose closed the socket under it.
+                }
+            })
+            { IsBackground = true, Name = $"fake-server-{Port}-{which}" }.Start();
+        }
+    }
+
+    public void Dispose()
+    {
+        lock (_lock)
+        {
+            _disposed = true;
+            foreach (Socket socket in _accepted)
+            {
+                socket.Dispose();
+            }
+
+            _accepted.Clear();
+        }
+
+        _listener.Dispose();
+    }
+}

tests/Ioxide.Tests.Redis/Program.cs

@@ -18,6 +18,7 @@ private static int Main()
         Console.WriteLine($"redis {(up ? "up" : "down")} ({redis.Host}:{redis.Port})\n");
 
         RedisTests.Register(runner, redis, up);
+        RedisReplyTests.Register(runner);
         return runner.Summary();
     }
 }

tests/Ioxide.Tests.Redis/RedisReplyTests.cs

@@ -0,0 +1,82 @@
+using System.Net.Sockets;
+using System.Text;
+using ioxide;
+using ioxide.redis;
+
+namespace Ioxide.Tests;
+
+/// <summary>Replies from a fake server that nests arrays without end. Needs no redis.</summary>
+internal static class RedisReplyTests
+{
+    public static void Register(Runner runner)
+    {
+        // The parser recursed once per level, so a deep enough reply overflowed the reactor's stack and killed the process.
+        runner.Test("redis: a reply nested 100,000 arrays deep fails the command instead of overflowing the stack", () =>
+        {
+            string outcome = Ping(100_000);
+            Assert.True(outcome.StartsWith("RedisException") && outcome.Contains("deep"),
+                $"expected a RedisException about the nesting, got [{outcome}]");
+        });
+
+        runner.Test("control: the same fake server's reply nested 8 arrays deep is parsed", () =>
+        {
+            Assert.Equal("depth 8", Ping(8));
+        });
+    }
+
+    // PINGs a fake server that answers with :1 inside depth one-element arrays, and reports the depth it parsed.
+    private static string Ping(int depth)
+    {
+        byte[] reply = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat("*1\r\n", depth)) + ":1\r\n");
+        using var fake = new FakeServer((_, socket) => Answer(socket, reply));
+        var options = new RedisOptions { Port = (ushort)fake.Port };
+
+        int port = TestServer.Start((r, conn) => Handle(r, conn, options));
+        (int status, string body) = Client.Get(port, "/ping");
+        Assert.Equal(200, status);
+        return body;
+    }
+
+    private static void Answer(Socket socket, byte[] reply)
+    {
+        FakeServer.Receive(socket, "*1\r\n$4\r\nPING\r\n".Length);
+        socket.Send(reply);
+    }
+
+    private static async Task Handle(Reactor r, TcpConnection conn, RedisOptions options)
+    {
+        try
+        {
+            RecvSnapshot snapshot = await conn.ReadAsync();
+            if (Wire.ReadPath(conn, snapshot) != "/ping")
+            {
+                return;   // the harness's listen probe
+            }
+
+            string outcome;
+            try
+            {
+                using RedisConnection redis = await RedisConnection.ConnectAsync(r, options);
+                RespValue reply = await redis.ExecuteAsync("PING");
+                int depth = 0;
+                for (; reply.Kind == RespKind.Array; depth++)
+                {
+                    reply = reply.Items[0];
+                }
+
+                outcome = $"depth {depth}";
+            }
+            catch (Exception e)
+            {
+                outcome = $"{e.GetType().Name}: {e.Message}";
+            }
+
+            Wire.Write(conn, 200, outcome);
+            await conn.FlushAsync();
+        }
+        finally
+        {
+            conn.DecRef();
+        }
+    }
+}