ARCHIVE/xahaud
3
0
Fork 0
mirror of https://github.com/Xahau/xahaud.git synced 2026-06-02 16:26:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(export): harden collector duplicate and identity handling

Browse Source
This commit is contained in:
Nicholas Dudfield
2026-03-02 15:25:19 +07:00
parent b99c38c09d
commit 07d741cdd7
2 changed files with 283 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

188
src/test/app/ExportSignatureCollector_test.cpp Normal file
View File

@@ -0,0 +1,188 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2026 XRPL Labs
Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================
#include <test/jtx.h>
#include <xrpld/app/misc/ExportSignatureCollector.h>
#include <xrpl/protocol/Sign.h>
namespace ripple {
namespace test {
class ExportSignatureCollector_test : public beast::unit_test::suite
{
static STTx
makeUnsignedTx()
{
auto const txKey = randomKeyPair(KeyType::secp256k1);
auto const txAccount = calcAccountID(txKey.first);
return STTx(ttACCOUNT_SET, [&txAccount, &txKey](auto& obj) {
obj.setAccountID(sfAccount, txAccount);
obj.setFieldVL(sfMessageKey, txKey.first.slice());
obj.setFieldVL(sfSigningPubKey, Slice{});
});
}
static STObject
makeSigner(PublicKey const& pk, AccountID const& acc, Blob const& sig)
{
STObject signer(sfSigner);
signer.setAccountID(sfAccount, acc);
signer.setFieldVL(sfSigningPubKey, pk.slice());
signer.setFieldVL(sfTxnSignature, sig);
return signer;
}
void
testDuplicateCanReplaceUnverified()
{
testcase("duplicate replaces unverified");
beast::Journal journal{beast::Journal::getNullSink()};
ExportSignatureCollector collector{journal};
auto const validator = randomKeyPair(KeyType::secp256k1);
auto const validatorAcc = calcAccountID(validator.first);
auto tx = makeUnsignedTx();
auto const txnHash = tx.getTransactionID();
Serializer txData;
tx.add(txData);
Serializer sigData = buildMultiSigningData(tx, validatorAcc);
auto const goodSigBuf =
sign(validator.first, validator.second, sigData.slice());
Blob goodSig(goodSigBuf.begin(), goodSigBuf.end());
Blob badSig = goodSig;
badSig.back() ^= 0x01;
auto badSigner = makeSigner(validator.first, validatorAcc, badSig);
auto goodSigner = makeSigner(validator.first, validatorAcc, goodSig);
BEAST_EXPECT(collector.verifyAndAddSignature(
txnHash, validator.first, badSigner, 100));
BEAST_EXPECT(!collector.isSignatureVerified(txnHash, validator.first));
collector.stashTxnData(txnHash, txData);
BEAST_EXPECT(collector.verifyAndAddSignature(
txnHash, validator.first, goodSigner, 101));
BEAST_EXPECT(collector.isSignatureVerified(txnHash, validator.first));
BEAST_EXPECT(collector.verifySignature(txnHash, validator.first));
auto const stored =
collector.getSignatureFrom(txnHash, validator.first);
BEAST_EXPECT(stored);
if (stored)
BEAST_EXPECT(stored->getFieldVL(sfTxnSignature) == goodSig);
}
void
testVerifiedIsNotReplaced()
{
testcase("verified is stable");
beast::Journal journal{beast::Journal::getNullSink()};
ExportSignatureCollector collector{journal};
auto const validator = randomKeyPair(KeyType::secp256k1);
auto const validatorAcc = calcAccountID(validator.first);
auto tx = makeUnsignedTx();
auto const txnHash = tx.getTransactionID();
Serializer txData;
tx.add(txData);
collector.stashTxnData(txnHash, txData);
Serializer sigData = buildMultiSigningData(tx, validatorAcc);
auto const goodSigBuf =
sign(validator.first, validator.second, sigData.slice());
Blob goodSig(goodSigBuf.begin(), goodSigBuf.end());
Blob badSig = goodSig;
badSig.back() ^= 0x01;
auto goodSigner = makeSigner(validator.first, validatorAcc, goodSig);
auto badSigner = makeSigner(validator.first, validatorAcc, badSig);
BEAST_EXPECT(collector.verifyAndAddSignature(
txnHash, validator.first, goodSigner, 200));
BEAST_EXPECT(collector.isSignatureVerified(txnHash, validator.first));
BEAST_EXPECT(collector.verifyAndAddSignature(
txnHash, validator.first, badSigner, 201));
BEAST_EXPECT(collector.verifySignature(txnHash, validator.first));
auto const stored =
collector.getSignatureFrom(txnHash, validator.first);
BEAST_EXPECT(stored);
if (stored)
BEAST_EXPECT(stored->getFieldVL(sfTxnSignature) == goodSig);
}
void
testRejectsSignerIdentityMismatch()
{
testcase("reject signer identity mismatch");
beast::Journal journal{beast::Journal::getNullSink()};
ExportSignatureCollector collector{journal};
auto const validatorA = randomKeyPair(KeyType::secp256k1);
auto const validatorB = randomKeyPair(KeyType::secp256k1);
auto const validatorBAcc = calcAccountID(validatorB.first);
auto tx = makeUnsignedTx();
auto const txnHash = tx.getTransactionID();
Serializer txData;
tx.add(txData);
collector.stashTxnData(txnHash, txData);
Serializer sigData = buildMultiSigningData(tx, validatorBAcc);
auto const sigBuf =
sign(validatorB.first, validatorB.second, sigData.slice());
Blob sig(sigBuf.begin(), sigBuf.end());
auto mismatchedSigner =
makeSigner(validatorB.first, validatorBAcc, sig);
BEAST_EXPECT(!collector.verifyAndAddSignature(
txnHash, validatorA.first, mismatchedSigner, 300));
BEAST_EXPECT(!collector.hasSignatureFrom(txnHash, validatorA.first));
BEAST_EXPECT(!collector.hasSignatureFrom(txnHash, validatorB.first));
}
public:
void
run() override
{
testDuplicateCanReplaceUnverified();
testVerifiedIsNotReplaced();
testRejectsSignerIdentityMismatch();
}
};
BEAST_DEFINE_TESTSUITE(ExportSignatureCollector, app, ripple);
} // namespace test
} // namespace ripple

121
src/xrpld/app/misc/detail/ExportSignatureCollector.cpp
View File

@@ -335,35 +335,60 @@ ExportSignatureCollector::verifyAndAddSignature(
<< " at ledger " << currentSeq;
}
// Check if we already have this signature
auto& signerMap = signatures_[txnHash];
if (signerMap.find(validator) != signerMap.end())
// The signer payload must bind to the validator who carried it in
// TMValidation. Otherwise a peer can misattribute signatures.
if (!signer.isFieldPresent(sfSigningPubKey) ||
!signer.isFieldPresent(sfAccount) ||
!signer.isFieldPresent(sfTxnSignature))
{
JLOG(j_.trace()) << "Export: already have signature from "
<< toBase58(TokenType::NodePublic, validator)
<< " for " << txnHash;
return true; // Already have it
JLOG(j_.warn()) << "Export: malformed signer payload for " << txnHash
<< " from "
<< toBase58(TokenType::NodePublic, validator);
return false;
}
// Try to verify if we have the txn data
bool verified = false;
auto txnIt = exportedTxnData_.find(txnHash);
if (txnIt != exportedTxnData_.end())
auto const sigPubKey = signer.getFieldVL(sfSigningPubKey);
if (sigPubKey.empty() || !publicKeyType(makeSlice(sigPubKey)))
{
JLOG(j_.warn()) << "Export: invalid signer pubkey payload for "
<< txnHash << " from "
<< toBase58(TokenType::NodePublic, validator);
return false;
}
auto const signingAcc = signer.getAccountID(sfAccount);
PublicKey const signerPubKey{makeSlice(sigPubKey)};
if (signerPubKey != validator || signingAcc != calcAccountID(validator))
{
JLOG(j_.warn()) << "Export: signer identity mismatch for " << txnHash
<< " from "
<< toBase58(TokenType::NodePublic, validator);
return false;
}
auto& signerMap = signatures_[txnHash];
// Verify if we have stashed tx data. Returns:
// true -> verified
// false -> verification failed
// nullopt -> cannot verify yet (no tx data)
auto verifyWithStashedData =
[&](STObject const& candidate) -> std::optional<bool> {
auto txnIt = exportedTxnData_.find(txnHash);
if (txnIt == exportedTxnData_.end())
return std::nullopt;
try
{
// Parse the stashed transaction
SerialIter sit(txnIt->second.slice());
auto stpTrans = std::make_shared<STTx const>(std::ref(sit));
// Get signer account from the signer object
auto signingAcc = signer.getAccountID(sfAccount);
auto sigPubKey = signer.getFieldVL(sfSigningPubKey);
auto signature = signer.getFieldVL(sfTxnSignature);
auto signingAcc = candidate.getAccountID(sfAccount);
auto sigPubKey = candidate.getFieldVL(sfSigningPubKey);
auto signature = candidate.getFieldVL(sfTxnSignature);
// Build the multisig data and verify
Serializer sigData = buildMultiSigningData(*stpTrans, signingAcc);
verified = ripple::verify(
bool const verified = ripple::verify(
PublicKey(makeSlice(sigPubKey)),
sigData.slice(),
makeSlice(signature),
@@ -374,39 +399,83 @@ ExportSignatureCollector::verifyAndAddSignature(
JLOG(j_.warn())
<< "Export: signature verification FAILED for " << txnHash
<< " from " << toBase58(TokenType::NodePublic, validator);
return false; // Don't add invalid signature
return false;
}
JLOG(j_.trace())
<< "Export: signature verified for " << txnHash << " from "
<< toBase58(TokenType::NodePublic, validator);
return true;
}
catch (std::exception const& e)
{
JLOG(j_.warn()) << "Export: signature verification exception for "
<< txnHash << ": " << e.what();
return false; // Don't add if we can't verify
return false;
}
};
auto verIt = verified_.find(txnHash);
bool const alreadyVerified = verIt != verified_.end() &&
verIt->second.find(validator) != verIt->second.end();
// Duplicate from same validator:
// - keep existing once cryptographically verified
// - allow replacement while unverified so a stale/bad early copy can heal
if (auto existing = signerMap.find(validator); existing != signerMap.end())
{
if (alreadyVerified)
{
JLOG(j_.trace())
<< "Export: ignoring duplicate verified signature "
<< "from " << toBase58(TokenType::NodePublic, validator)
<< " for " << txnHash;
return true;
}
auto const verified = verifyWithStashedData(signer);
if (verified && !*verified)
return false; // Reject replacement with invalid signature
if (!verified)
{
JLOG(j_.trace())
<< "Export: replacing unverified signature for " << txnHash
<< " from " << toBase58(TokenType::NodePublic, validator)
<< " (no txn data yet)";
}
existing->second = std::move(signer);
if (verified && *verified)
verified_[txnHash].insert(validator);
JLOG(j_.trace()) << "Export: replaced signature from "
<< toBase58(TokenType::NodePublic, validator)
<< " for " << txnHash
<< " (verified=" << (verified && *verified) << ")";
return true;
}
else
auto const verified = verifyWithStashedData(signer);
if (verified && !*verified)
return false; // Don't add invalid signature
if (!verified)
{
// No txn data yet - add unverified (will verify later or in Transactor)
JLOG(j_.trace()) << "Export: adding unverified signature for "
<< txnHash << " (no txn data yet)";
}
// Add the signature
signerMap.emplace(validator, std::move(signer));
if (verified)
{
if (verified && *verified)
verified_[txnHash].insert(validator);
}
JLOG(j_.trace()) << "Export: added signature from "
<< toBase58(TokenType::NodePublic, validator) << " for "
<< txnHash << " (total: " << signerMap.size()
<< ", verified=" << verified << ")";
<< ", verified=" << (verified && *verified) << ")";
return true;
}