blindpsbt: require genuine commitments in VerifyBlindValueProof
What the developer wrote
Authored by Byron Hambly
blindpsbt: require genuine commitments in VerifyBlindValueProof
An explicit 9-byte value (or a null field) passed the IsNull() check and
its buffer was then parsed as a 33-byte Pedersen commitment, reading past
the end. Require IsCommitment() on both the value and asset fields so the
parser's length precondition holds and the out-of-bounds read is avoided.
What changed, and why it matters
This commit fixes a bug in the way confidential transaction value proofs are validated. Previously, the code accepted an explicit 9-byte value or an empty field where a 33-byte cryptographic commitment was expected. It then passed that shorter buffer to a library that always reads 33 bytes, causing it to read past the end of the data. The fix now requires both the value and asset fields to be genuine 33-byte commitments before handing them to the parser, preventing an out-of-bounds read.
Apply this patch. Review other call sites that pass confidential value/asset buffers to libsecp256k1 to ensure they also validate IsCommitment() before use. Consider fuzzing VerifyBlindValueProof with explicit and null confidential value/asset inputs.
Security signals we found
Out-of-bounds read in cryptographic proof verification
Insufficient input validation before passing buffer to libsecp256k1
Confidential value/asset commitment type confusion
Potential memory safety issue in blind PSBT handling
Evidence from the diff
In VerifyBlindValueProof(), the precondition was changed from IsNull() to IsCommitment() for both conf_value and conf_asset. The old check allowed explicit 9-byte CConfidentialValue objects (or null/empty fields) to pass, after which the buffer was treated as a 33-byte Pedersen commitment and fed to libsecp256k1’s value proof verification, resulting in an out-of-bounds read of up to 24 bytes. The new check ensures the buffers are exactly 33-byte commitments (PrefixA/B) so the parser’s length precondition holds.
Changed components
src/blindpsbt.cppVerifyBlindValueProof()CConfidentialValueCConfidentialAssetInspect captured patch +5 / −1
### src/blindpsbt.cpp
@@ -189,7 +189,11 @@ bool CreateBlindAssetProof(std::vector<unsigned char>& assetproof, const CAsset&
bool VerifyBlindValueProof(CAmount value, const CConfidentialValue& conf_value, const std::vector<unsigned char>& proof, const CConfidentialAsset& conf_asset)
{
- if (conf_value.IsNull() || conf_asset.IsNull()) {
+ // The value and asset must be genuine commitments (33-byte, PrefixA/B)
+ // before their buffers are handed to libsecp256k1, which consumes exactly
+ // 33 serialized bytes. An explicit 9-byte value (or a null field) must not
+ // reach the parser, which would otherwise read out of bounds.
+ if (!conf_value.IsCommitment() || !conf_asset.IsCommitment()) {
return false;
}
Why this scored 62/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.