switch electrum server notification detection to streaming json token parse
What changed, and why it matters
This commit changes how Sparrow Wallet decides whether a message from an Electrum server is a subscription notification. Previously it used a simple text search (looking for the word 'method' and absence of the word 'error'), which could be fooled by JSON strings containing those words. Now it uses a proper JSON parser to check whether the message actually has a 'method' field at the top level. This is a hardening fix: the old approach could misclassify server responses, possibly causing user notifications to be missed or processed incorrectly. There is no direct evidence in the commit that this was exploited or treated as a security bug by the vendor.
Treat as a defensive hardening improvement. Users should upgrade to a build containing this commit. Developers should review whether the old heuristic caused any observable misbehavior (missed block/address subscriptions) and consider whether additional input validation is needed on the jsonRpcServer.handle path. No emergency response is indicated by the diff alone.
Security signals we found
Replaced substring-based JSON classification with streaming token parsing
Added robust handling of malformed JSON with try/catch around JsonParser
Top-level field validation prevents nested 'method' strings from triggering notification handling
Potential for missed or spoofed wallet notifications under the old heuristic
Evidence from the diff
TcpTransport previously classified inbound Electrum JSON-RPC traffic as a notification if the raw string contained ‘method’ and did not contain ‘error’. This heuristic is fragile: a response result/error string that happens to include the substring ‘method’ could be treated as a notification, and a notification whose payload or method name contains the substring ‘error’ could be treated as a response. The patch replaces the heuristic with a streaming Jackson JsonParser that validates the top-level JSON object and returns true only when a top-level ‘method’ field exists and its value is a JSON string. The parser also skips nested children safely. This reduces misclassification and makes notification detection robust against adversarial or unusual JSON content.
Changed components
src/main/java/com/sparrowwallet/sparrow/net/TcpTransport.javaElectrum server TCP transport layerJSON-RPC notification dispatchInspect captured patch +25 / −3
diff --git a/src/main/java/com/sparrowwallet/sparrow/net/TcpTransport.java b/src/main/java/com/sparrowwallet/sparrow/net/TcpTransport.java
index 99da1f7..1f7d052 100644
--- a/src/main/java/com/sparrowwallet/sparrow/net/TcpTransport.java
+++ b/src/main/java/com/sparrowwallet/sparrow/net/TcpTransport.java
@@ -1,5 +1,8 @@
package com.sparrowwallet.sparrow.net;
+import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonToken;
import com.github.arteam.simplejsonrpc.server.JsonRpcServer;
import com.google.common.base.Splitter;
import com.google.common.net.HostAndPort;
@@ -37,6 +40,7 @@ public class TcpTransport implements CloseableTransport, TimeoutCounter {
public static final long PER_REQUEST_READ_TIMEOUT_MILLIS = 50;
public static final int SOCKET_READ_TIMEOUT_MILLIS = 5000;
private static final Pattern ID_PATTERN = Pattern.compile("\"id\"\\s*:\\s*(\\d+)");
+ private static final JsonFactory JSON_FACTORY = new JsonFactory();
protected final HostAndPort server;
protected final SocketFactory socketFactory;
@@ -201,11 +205,9 @@ public class TcpTransport implements CloseableTransport, TimeoutCounter {
try {
String received = readInputStream(in);
wireLog.info("< " + received);
- if(received.contains("method") && !received.contains("error")) {
- //Handle subscription notification
+ if(isNotification(received)) {
jsonRpcServer.handle(received, subscriptionService);
} else {
- //Handle client's response
response = received;
reading = false;
readingCondition.signal();
@@ -312,6 +314,26 @@ public class TcpTransport implements CloseableTransport, TimeoutCounter {
return readTimeoutIndex;
}
+ private static boolean isNotification(String json) {
+ try(JsonParser parser = JSON_FACTORY.createParser(json)) {
+ if(parser.nextToken() != JsonToken.START_OBJECT) {
+ return false;
+ }
+ while(parser.nextToken() == JsonToken.FIELD_NAME) {
+ String field = parser.currentName();
+ JsonToken value = parser.nextToken();
+ if("method".equals(field)) {
+ return value == JsonToken.VALUE_STRING;
+ }
+ parser.skipChildren();
+ }
+ return false;
+ } catch(Exception e) {
+ log.warn("Could not parse JSON-RPC message from server: " + e.getMessage());
+ return false;
+ }
+ }
+
private static Set<String> extractIdSet(String json) {
if(json == null || json.isEmpty()) {
return Collections.emptySet();
Why this scored 49/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.