fix: Compute validation suppression key over canonical serialisation

This commit is contained in:
Jingchen
2026-07-17 12:33:44 +01:00
committed by Ayaz Salikhov
parent 7877ee42a0
commit 7d3611df2a
5 changed files with 95 additions and 26 deletions

View File

@@ -56,10 +56,15 @@ STObject::STObject(SOTemplate const& type, SField const& name) : STBase(name)
set(type);
}
STObject::STObject(SOTemplate const& type, SerialIter& sit, SField const& name) : STBase(name)
STObject::STObject(
SOTemplate const& type,
SerialIter& sit,
SField const& name,
bool requireCanonicalOrder)
: STBase(name)
{
v_.reserve(type.size());
set(sit);
set(sit, 0, requireCanonicalOrder);
applyTemplate(type); // May throw
}
@@ -208,12 +213,13 @@ STObject::applyTemplateFromSField(SField const& sField)
// return true = terminated with end-of-object
bool
STObject::set(SerialIter& sit, int depth)
STObject::set(SerialIter& sit, int depth, bool requireCanonicalOrder)
{
bool reachedEndOfObject = false;
v_.clear();
std::optional<int> prevFieldCode;
// Consume data in the pipe until we run out or reach the end
while (!sit.empty())
{
@@ -238,7 +244,6 @@ STObject::set(SerialIter& sit, int depth)
}
auto const& fn = SField::getField(type, field);
if (fn.isInvalid())
{
JLOG(debugLog().error())
@@ -246,6 +251,13 @@ STObject::set(SerialIter& sit, int depth)
Throw<std::runtime_error>("Unknown field");
}
if (requireCanonicalOrder && prevFieldCode.has_value() && fn.fieldCodeMem <= *prevFieldCode)
{
JLOG(debugLog().error()) << "Fields in object are not in canonical order";
Throw<std::runtime_error>("Fields in object are not in canonical order");
}
prevFieldCode = fn.fieldCodeMem;
// Unflatten the field
v_.emplace_back(sit, fn, depth + 1);

View File

@@ -153,7 +153,10 @@ public:
SerialIter sit{kPayload8};
auto val = std::make_shared<STValidation>(
sit, [](PublicKey const& pk) { return calcNodeID(pk); }, true);
sit,
[](PublicKey const& pk) { return calcNodeID(pk); },
STValidation::DeserializeOptions{
.checkSignature = true, .requireCanonicalOrder = false});
BEAST_EXPECT(val);
BEAST_EXPECT(val->isFieldPresent(sfLedgerSequence));
@@ -174,7 +177,10 @@ public:
{
SerialIter sit{kPayload1};
auto val = std::make_shared<xrpl::STValidation>(
sit, [](PublicKey const& pk) { return calcNodeID(pk); }, false);
sit,
[](PublicKey const& pk) { return calcNodeID(pk); },
STValidation::DeserializeOptions{
.checkSignature = false, .requireCanonicalOrder = false});
fail("An exception should have been thrown");
}
catch (std::exception const& ex)
@@ -186,7 +192,10 @@ public:
{
SerialIter sit{kPayload2};
auto val = std::make_shared<xrpl::STValidation>(
sit, [](PublicKey const& pk) { return calcNodeID(pk); }, false);
sit,
[](PublicKey const& pk) { return calcNodeID(pk); },
STValidation::DeserializeOptions{
.checkSignature = false, .requireCanonicalOrder = false});
fail("An exception should have been thrown");
}
catch (std::exception const& ex)
@@ -198,7 +207,10 @@ public:
{
SerialIter sit{kPayload3};
auto val = std::make_shared<xrpl::STValidation>(
sit, [](PublicKey const& pk) { return calcNodeID(pk); }, false);
sit,
[](PublicKey const& pk) { return calcNodeID(pk); },
STValidation::DeserializeOptions{
.checkSignature = false, .requireCanonicalOrder = false});
fail("An exception should have been thrown");
}
catch (std::exception const& ex)
@@ -210,7 +222,10 @@ public:
{
SerialIter sit{kPayload4};
auto val = std::make_shared<xrpl::STValidation>(
sit, [](PublicKey const& pk) { return calcNodeID(pk); }, false);
sit,
[](PublicKey const& pk) { return calcNodeID(pk); },
STValidation::DeserializeOptions{
.checkSignature = false, .requireCanonicalOrder = false});
fail("An exception should have been thrown");
}
catch (std::exception const& ex)
@@ -224,7 +239,10 @@ public:
{
SerialIter sit{kPayload5};
auto val = std::make_shared<STValidation>(
sit, [](PublicKey const& pk) { return calcNodeID(pk); }, false);
sit,
[](PublicKey const& pk) { return calcNodeID(pk); },
STValidation::DeserializeOptions{
.checkSignature = false, .requireCanonicalOrder = false});
fail("Expected exception not thrown from validation");
}
catch (std::exception const& ex)
@@ -236,7 +254,10 @@ public:
{
SerialIter sit{kPayload6};
auto val = std::make_shared<STValidation>(
sit, [](PublicKey const& pk) { return calcNodeID(pk); }, false);
sit,
[](PublicKey const& pk) { return calcNodeID(pk); },
STValidation::DeserializeOptions{
.checkSignature = false, .requireCanonicalOrder = false});
fail("Expected exception not thrown from validation");
}
catch (std::exception const& ex)
@@ -249,7 +270,10 @@ public:
SerialIter sit{kPayload7};
auto val = std::make_shared<STValidation>(
sit, [](PublicKey const& pk) { return calcNodeID(pk); }, false);
sit,
[](PublicKey const& pk) { return calcNodeID(pk); },
STValidation::DeserializeOptions{
.checkSignature = false, .requireCanonicalOrder = false});
fail("Expected exception not thrown from validation");
}
@@ -279,7 +303,10 @@ public:
SerialIter sit{makeSlice(v2)};
auto val = std::make_shared<STValidation>(
sit, [](PublicKey const& pk) { return calcNodeID(pk); }, true);
sit,
[](PublicKey const& pk) { return calcNodeID(pk); },
STValidation::DeserializeOptions{
.checkSignature = true, .requireCanonicalOrder = false});
fail("Mutated validation signature checked out: offset=" + std::to_string(i));
}

View File

@@ -2346,12 +2346,22 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMValidation> const& m)
std::shared_ptr<STValidation> val;
{
SerialIter sit(makeSlice(m->validation()));
val = std::make_shared<STValidation>(
std::ref(sit),
[this](PublicKey const& pk) {
return calcNodeID(app_.getValidatorManifests().getMasterKey(pk));
},
false);
try
{
val = std::make_shared<STValidation>(
std::ref(sit),
[this](PublicKey const& pk) {
return calcNodeID(app_.getValidatorManifests().getMasterKey(pk));
},
STValidation::DeserializeOptions{
.checkSignature = false, .requireCanonicalOrder = true});
}
catch (std::exception const& e)
{
JLOG(pJournal_.warn()) << "Validation: Exception, " << e.what();
fee_.update(Resource::kFeeInvalidData, e.what());
return;
}
val->setSeen(closeTime);
}