What the developer wrote
Authored by jeffro256
ringct: make h2d fallible
One theoretically could have crafted a "long amount" (pre-v10) RingCT transaction
with non-0 padding bytes in the decoded amount which fails receiver scanning, but
passes third-party wallet proof checking.
What changed, and why it matters
This Monero patch fixes a RingCT amount-decoding function that previously ignored extra bytes beyond the first 8 in a 32-byte amount field. A pre-v10 'long amount' transaction could hide non-zero padding after the real amount. The receiver's wallet would fail to decode it, but a third-party wallet checking a proof might accept it, creating a mismatch between what the sender proves and what the recipient can actually spend. The change makes the decoder reject such malformed amounts.
Treat as a security fix. Review whether pre-v10 long-amount transaction validation on the network already rejects such padding, or whether a consensus/validation rule is also needed in addition to this wallet-level decoder change. Backport to maintained branches if applicable and consider a security advisory.
Security signals we found
Amount-decoding function silently ignored 24 bytes of input
Patch makes decoder reject non-zero high bytes
Commit message describes crafted transaction that can fool third-party proof verification
Receiver scanning failure vs. third-party proof success implies proof/verification inconsistency
Pre-v10 RingCT long-amount format is the affected transaction type
Evidence from the diff
The h2d() helper converts a 32-byte rct::key to a 64-bit xmr_amount. Before the patch it read only the low 8 bytes and silently discarded the remaining 24 bytes. The patch changes h2d() to return bool and fail if any of bytes 8-31 are non-zero. Callers in rctSigs.cpp (decodeRct, decodeRctSimple) and the multisig test now throw/check on failure. The commit message explicitly frames this as a fix for crafted pre-v10 long-amount RingCT transactions whose padding bytes pass third-party proof checking while failing receiver scanning.
Changed components
src/ringct/rctTypes.cppsrc/ringct/rctTypes.hsrc/ringct/rctSigs.cppsrc/ringct/rctOps.cppsrc/ringct/rctOps.htests/core_tests/multisig.cpptests/unit_tests/ringct.cppInspect captured patch +41 / −30
diff --git a/src/ringct/rctOps.cpp b/src/ringct/rctOps.cpp
index 3860a7e..34d6403 100644
--- a/src/ringct/rctOps.cpp
+++ b/src/ringct/rctOps.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2016-2024, Monero Research Labs
+// Copyright (c) 2016-2026, Monero Research Labs
//
// Author: Shen Noether <shen.noether@gmx.com>
//
@@ -30,7 +30,11 @@
#include <boost/lexical_cast.hpp>
#include "misc_log_ex.h"
+extern "C" {
+#include "rctCryptoOps.h"
+}
#include "rctOps.h"
+
using namespace crypto;
using namespace std;
@@ -340,7 +344,8 @@ namespace rct {
//generates a random uint long long (for testing)
xmr_amount randXmrAmount(xmr_amount upperlimit) {
- return h2d(skGen()) % (upperlimit);
+ assert(upperlimit > 0);
+ return crypto::rand<xmr_amount>() % upperlimit;
}
//Scalar multiplications of curve points
diff --git a/src/ringct/rctOps.h b/src/ringct/rctOps.h
index 0edd030..88aad0e 100644
--- a/src/ringct/rctOps.h
+++ b/src/ringct/rctOps.h
@@ -1,5 +1,5 @@
//#define DBG
-// Copyright (c) 2016-2024, Monero Research Labs
+// Copyright (c) 2016-2026, Monero Research Labs
//
// Author: Shen Noether <shen.noether@gmx.com>
//
@@ -37,15 +37,6 @@
#include <cstddef>
#include <tuple>
-#include "crypto/generic-ops.h"
-
-extern "C" {
-#include "crypto/random.h"
-#include "crypto/keccak.h"
-#include "rctCryptoOps.h"
-}
-#include "crypto/crypto.h"
-
#include "rctTypes.h"
//Define this flag when debugging to get additional info on the console
diff --git a/src/ringct/rctSigs.cpp b/src/ringct/rctSigs.cpp
index 206a9a5..08e47cc 100644
--- a/src/ringct/rctSigs.cpp
+++ b/src/ringct/rctSigs.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2016-2024, Monero Research Labs
+// Copyright (c) 2016-2026, Monero Research Labs
//
// Author: Shen Noether <shen.noether@gmx.com>
//
@@ -1573,7 +1573,10 @@ namespace rct {
if (equalKeys(C, Ctmp) == false) {
CHECK_AND_ASSERT_THROW_MES(false, "warning, amount decoded incorrectly, will be unable to spend");
}
- return h2d(amount);
+ rct::xmr_amount amount_8;
+ CHECK_AND_ASSERT_THROW_MES(h2d(amount_8, amount),
+ "long decoded amount contains superfluous data");
+ return amount_8;
}
xmr_amount decodeRct(const rctSig & rv, const key & sk, unsigned int i, hw::device &hwdev) {
@@ -1604,7 +1607,10 @@ namespace rct {
if (equalKeys(C, Ctmp) == false) {
CHECK_AND_ASSERT_THROW_MES(false, "warning, amount decoded incorrectly, will be unable to spend");
}
- return h2d(amount);
+ rct::xmr_amount amount_8;
+ CHECK_AND_ASSERT_THROW_MES(h2d(amount_8, amount),
+ "long decoded amount contains superfluous data");
+ return amount_8;
}
xmr_amount decodeRctSimple(const rctSig & rv, const key & sk, unsigned int i, hw::device &hwdev) {
diff --git a/src/ringct/rctTypes.cpp b/src/ringct/rctTypes.cpp
index 8b0345e..51260c0 100644
--- a/src/ringct/rctTypes.cpp
+++ b/src/ringct/rctTypes.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2016-2024, Monero Research Labs
+// Copyright (c) 2016-2026, Monero Research Labs
//
// Author: Shen Noether <shen.noether@gmx.com>
//
@@ -140,14 +140,18 @@ namespace rct {
//32 byte key to uint long long
// if the key holds a value > 2^64
- // then the value in the first 8 bytes is returned
- xmr_amount h2d(const key & test) {
- xmr_amount vali = 0;
+ // then false is returned
+ bool h2d(xmr_amount &amountd, const key & test) {
+ amountd = 0;
int j = 0;
+ for (j = 8; j < 32; ++j) {
+ if (test.bytes[j])
+ return false;
+ }
for (j = 7; j >= 0; j--) {
- vali = (xmr_amount)(vali * 256 + (unsigned char)test.bytes[j]);
+ amountd = (xmr_amount)(amountd * 256 + (unsigned char)test.bytes[j]);
}
- return vali;
+ return true;
}
//32 byte key to int[64]
diff --git a/src/ringct/rctTypes.h b/src/ringct/rctTypes.h
index ee896f1..038c2e1 100644
--- a/src/ringct/rctTypes.h
+++ b/src/ringct/rctTypes.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2016-2024, Monero Research Labs
+// Copyright (c) 2016-2026, Monero Research Labs
//
// Author: Shen Noether <shen.noether@gmx.com>
//
@@ -722,8 +722,8 @@ namespace rct {
void d2b(bits amountb, xmr_amount val);
//32 byte key to uint long long
// if the key holds a value > 2^64
- // then the value in the first 8 bytes is returned
- xmr_amount h2d(const key &test);
+ // then false is returned
+ bool h2d(xmr_amount &amountd, const key &test);
//32 byte key to int[64]
void h2b(bits amountb2, const key & test);
//int[64] to 32 byte key
diff --git a/tests/core_tests/multisig.cpp b/tests/core_tests/multisig.cpp
index 9cec96f..c3874f9 100644
--- a/tests/core_tests/multisig.cpp
+++ b/tests/core_tests/multisig.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2017-2024, The Monero Project
+// Copyright (c) 2017-2026, The Monero Project
//
// All rights reserved.
//
@@ -438,7 +438,10 @@ bool gen_multisig_tx_validation_base::generate_with(std::vector<test_event_entry
rct::key C = tx.rct_signatures.outPk[n].mask;
rct::addKeys2(Ctmp, ecdh_info.mask, ecdh_info.amount, rct::H);
CHECK_AND_ASSERT_MES(rct::equalKeys(C, Ctmp), false, "Failed to decode amount");
- amount += rct::h2d(ecdh_info.amount);
+ rct::xmr_amount this_amount;
+ CHECK_AND_ASSERT_MES(rct::h2d(this_amount, ecdh_info.amount),
+ false, "Decoded long amount contains superfluous data");
+ amount += this_amount;
}
}
CHECK_AND_ASSERT_MES(n_outs == 2, false, "Not exactly 2 outputs were received");
diff --git a/tests/unit_tests/ringct.cpp b/tests/unit_tests/ringct.cpp
index 9979fad..e2476f4 100644
--- a/tests/unit_tests/ringct.cpp
+++ b/tests/unit_tests/ringct.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2014-2024, The Monero Project
+// Copyright (c) 2014-2026, The Monero Project
//
// All rights reserved.
//
@@ -974,11 +974,13 @@ static const xmr_amount test_amounts[]={0, 1, 2, 3, 4, 5, 10000, 100000000000000
TEST(ringct, d2h)
{
- key k, P1;
- skpkGen(k, P1);
+ key k = skGen();
+ memset(k.bytes + 8, 0, sizeof(k) - 8);
for (auto amount: test_amounts) {
d2h(k, amount);
- ASSERT_TRUE(amount == h2d(k));
+ xmr_amount h2d_amount;
+ ASSERT_TRUE(h2d(h2d_amount, k));
+ ASSERT_EQ(h2d_amount, amount);
}
}
Why this scored 65/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.