From d9c75eaec27fc92f82880f44dcd1626d469dc96b Mon Sep 17 00:00:00 2001 From: Davide Grilli Date: Tue, 7 Jul 2026 21:51:52 +0200 Subject: [PATCH] fix(net): ParsePeers must tolerate any JSON shape from the server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit server.peers.subscribe is attacker-controlled data: ParsePeers assumed the standard [ip, hostname, [features...]] shape and threw on anything else (non-array response, wrong element types), and GetString() on a JSON string containing invalid UTF-8 bytes threw InvalidOperationException instead of failing to parse — both found by fuzzing. Any unexpected shape or unparseable string now degrades to "no usable data" instead of propagating an exception into the sync/discovery path. --- src/Core/Net/ElectrumApi.cs | 30 ++++++++++++++++--- .../Net/ServerRegistryTests.cs | 29 ++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/Core/Net/ElectrumApi.cs b/src/Core/Net/ElectrumApi.cs index db423ea..18c46ef 100644 --- a/src/Core/Net/ElectrumApi.cs +++ b/src/Core/Net/ElectrumApi.cs @@ -124,17 +124,22 @@ public static class ElectrumApi /// Parses the server.peers.subscribe response: a list of /// [ip, hostname, ["v1.4.2", "pN", "tPORT", "sPORT", ...]]; /// "t"/"s" without a number = network default port (resolved by the caller). + /// The response is untrusted server data: any unexpected shape (non-array, + /// wrong element types) is skipped, never thrown on. /// public static IReadOnlyList ParsePeers(JsonElement response) { var peers = new List(); + if (response.ValueKind != JsonValueKind.Array) + return peers; foreach (var entry in response.EnumerateArray()) { - if (entry.ValueKind != JsonValueKind.Array || entry.GetArrayLength() < 3) + if (entry.ValueKind != JsonValueKind.Array || entry.GetArrayLength() < 3 + || entry[2].ValueKind != JsonValueKind.Array) continue; - var host = entry[1].GetString(); + var host = AsString(entry[1]); if (string.IsNullOrWhiteSpace(host)) - host = entry[0].GetString(); + host = AsString(entry[0]); if (string.IsNullOrWhiteSpace(host)) continue; @@ -142,7 +147,7 @@ public static class ElectrumApi string? version = null; foreach (var feature in entry[2].EnumerateArray()) { - var f = feature.GetString(); + var f = AsString(feature); if (string.IsNullOrEmpty(f)) continue; switch (f[0]) @@ -157,4 +162,21 @@ public static class ElectrumApi } return peers; } + + private static string? AsString(JsonElement element) + { + if (element.ValueKind != JsonValueKind.String) + return null; + try + { + return element.GetString(); + } + catch (InvalidOperationException) + { + // Syntactically valid JSON string that cannot be transcoded to UTF-16 + // (invalid UTF-8 bytes / lone surrogates) — untrusted server data, + // treat as absent (found by fuzzing). + return null; + } + } } diff --git a/tests/PalladiumWallet.Tests/Net/ServerRegistryTests.cs b/tests/PalladiumWallet.Tests/Net/ServerRegistryTests.cs index df24515..7054755 100644 --- a/tests/PalladiumWallet.Tests/Net/ServerRegistryTests.cs +++ b/tests/PalladiumWallet.Tests/Net/ServerRegistryTests.cs @@ -27,6 +27,35 @@ public class PeerParsingTests Assert.Equal(new PeerInfo("nodo.esempio.org", 0, null, "1.4"), peers[1]); Assert.Equal(new PeerInfo("solo-ssl.esempio.org", null, 50002, "1.4"), peers[2]); } + + [Theory] + [InlineData("""{"not":"an array"}""")] + [InlineData(""" "just a string" """)] + [InlineData("42")] + [InlineData("""[[1,2,3]]""")] // numeric ip/hostname + [InlineData("""[["h","n",42]]""")] // features not an array + [InlineData("""[["h","n",[42,{"x":1},null]]]""")] // non-string features + public void Una_risposta_peers_di_forma_inattesa_produce_una_lista_vuota(string json) + { + // The response is untrusted server data: any shape must degrade to an + // empty list, never throw (found by fuzzing). + Assert.Empty(ElectrumApi.ParsePeers(JsonDocument.Parse(json).RootElement)); + } + + [Fact] + public void Una_stringa_json_con_utf8_invalido_viene_ignorata_senza_eccezioni() + { + // Raw 0x87 inside a JSON string: JsonDocument.Parse accepts the bytes but + // GetString() cannot transcode them to UTF-16 (found by fuzzing). The + // whole entry degrades to "no usable host/feature", not an exception. + var bytes = "[[\"1.2.3.4\",\"#\",[\"v1\",\"t#\"]]]"u8.ToArray(); + var hostIdx = Array.IndexOf(bytes, (byte)'#'); + bytes[hostIdx] = 0x87; + bytes[Array.LastIndexOf(bytes, (byte)'#')] = 0x87; + + using var doc = JsonDocument.Parse(bytes); + Assert.Empty(ElectrumApi.ParsePeers(doc.RootElement)); + } } public class ServerRegistryTests