What changed, and why it matters
This commit changes how the btcd RPC client parses JSON string responses. Instead of using a full JSON parser to extract quoted strings, it now uses a custom helper that strips the surrounding quote characters directly. The stated goal is performance: fewer memory allocations and faster parsing. The change removes proper JSON validation from several code paths, which could in theory allow malformed or unexpected server responses to be processed incorrectly. However, the commit message frames this purely as an optimization, and no security bug or exploit is demonstrated in the materials.
Review the implementation of parseJSONString and parseJSONStringReader to confirm they at minimum reject inputs that are not valid JSON strings (e.g., verify leading/trailing double quotes, reject unescaped control characters, handle escaped quotes correctly). If the helper is only a trim, consider reintroducing JSON validation on untrusted inputs or documenting the trust assumption that the RPC server is local and trusted. Add unit tests covering edge cases such as escaped quotes, embedded quotes, and non-string JSON values.
Security signals we found
Removal of standard JSON parsing in favor of a custom string-stripping helper
Potential acceptance of malformed or non-JSON input if the helper is naive
Parsing of server-controlled hex data without intermediate validation
No visible security review, advisory, or CVE references in the commit or supplied materials
Evidence from the diff
The patch replaces json.Unmarshal(res, &someString) with parseJSONString(res) and in some places feeds the result into hex.NewDecoder(parseJSONStringReader(res)). This bypasses the standard library’s JSON string unescaping and validation. If parseJSONString is implemented as a naive prefix/suffix trim (as the commit message implies), it will accept inputs that are not valid JSON strings, such as unescaped control characters, mismatched quotes, or strings containing escaped quotes. The affected code paths parse block hashes, block hex, block headers, and committed filter data received from an RPC server. In a typical deployment the RPC server is a trusted local bitcoind/btcd, so the practical attack surface is limited. The commit does not include tests or documentation of parseJSONString’s behavior.
Changed components
rpcclient/chain.goFutureGetBestBlockHashResult.ReceiveFutureGetBlockResult.ReceiveFutureGetBlockHashResult.ReceiveFutureGetBlockHeaderResult.ReceiveFutureGetCFilterResult.ReceiveFutureGetCFilterHeaderResult.ReceiveInspect captured patch +6 / −57
diff --git a/rpcclient/chain.go b/rpcclient/chain.go
index 1721c92..878a400 100644
--- a/rpcclient/chain.go
+++ b/rpcclient/chain.go
@@ -29,13 +29,7 @@ func (r FutureGetBestBlockHashResult) Receive() (*chainhash.Hash, error) {
return nil, err
}
- // Unmarshal result as a string.
- var txHashStr string
- err = json.Unmarshal(res, &txHashStr)
- if err != nil {
- return nil, err
- }
- return chainhash.NewHashFromStr(txHashStr)
+ return chainhash.NewHashFromStr(parseJSONString(res))
}
// GetBestBlockHashAsync returns an instance of a type that can be used to get
@@ -113,22 +107,9 @@ func (r FutureGetBlockResult) Receive() (*wire.MsgBlock, error) {
return nil, err
}
- // Unmarshal result as a string.
- var blockHex string
- err = json.Unmarshal(res, &blockHex)
- if err != nil {
- return nil, err
- }
-
- // Decode the serialized block hex to raw bytes.
- serializedBlock, err := hex.DecodeString(blockHex)
- if err != nil {
- return nil, err
- }
-
// Deserialize the block and return it.
var msgBlock wire.MsgBlock
- err = msgBlock.Deserialize(bytes.NewReader(serializedBlock))
+ err = msgBlock.Deserialize(hex.NewDecoder(parseJSONStringReader(res)))
if err != nil {
return nil, err
}
@@ -560,13 +541,7 @@ func (r FutureGetBlockHashResult) Receive() (*chainhash.Hash, error) {
return nil, err
}
- // Unmarshal the result as a string-encoded sha.
- var txHashStr string
- err = json.Unmarshal(res, &txHashStr)
- if err != nil {
- return nil, err
- }
- return chainhash.NewHashFromStr(txHashStr)
+ return chainhash.NewHashFromStr(parseJSONString(res))
}
// GetBlockHashAsync returns an instance of a type that can be used to get the
@@ -597,21 +572,9 @@ func (r FutureGetBlockHeaderResult) Receive() (*wire.BlockHeader, error) {
return nil, err
}
- // Unmarshal result as a string.
- var bhHex string
- err = json.Unmarshal(res, &bhHex)
- if err != nil {
- return nil, err
- }
-
- serializedBH, err := hex.DecodeString(bhHex)
- if err != nil {
- return nil, err
- }
-
// Deserialize the blockheader and return it.
var bh wire.BlockHeader
- err = bh.Deserialize(bytes.NewReader(serializedBH))
+ err = bh.Deserialize(hex.NewDecoder(parseJSONStringReader(res)))
if err != nil {
return nil, err
}
@@ -1246,15 +1209,8 @@ func (r FutureGetCFilterResult) Receive() (*wire.MsgCFilter, error) {
return nil, err
}
- // Unmarshal result as a string.
- var filterHex string
- err = json.Unmarshal(res, &filterHex)
- if err != nil {
- return nil, err
- }
-
// Decode the serialized cf hex to raw bytes.
- serializedFilter, err := hex.DecodeString(filterHex)
+ serializedFilter, err := hex.DecodeString(parseJSONString(res))
if err != nil {
return nil, err
}
@@ -1301,15 +1257,8 @@ func (r FutureGetCFilterHeaderResult) Receive() (*wire.MsgCFHeaders, error) {
return nil, err
}
- // Unmarshal result as a string.
- var headerHex string
- err = json.Unmarshal(res, &headerHex)
- if err != nil {
- return nil, err
- }
-
// Assign the decoded header into a hash
- headerHash, err := chainhash.NewHashFromStr(headerHex)
+ headerHash, err := chainhash.NewHashFromStr(parseJSONString(res))
if err != nil {
return nil, err
}
Why this scored 25/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.