Fix DynamicMPT for Attackathon (#6834)

This commit is contained in:
yinyiqian1
2026-04-09 22:47:46 -04:00
committed by GitHub
parent 4e2b41c425
commit 7cdbbc4d90
3 changed files with 89 additions and 31 deletions

View File

@@ -335,9 +335,24 @@ requireAuth(
auto const maybeDomainID = sleIssuance->at(~sfDomainID);
if (maybeDomainID)
{
XRPL_ASSERT(
sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth,
"xrpl::requireAuth : issuance requires authorization");
// Defensive check: An issuance with sfDomainID must strictly enforce lsfMPTRequireAuth.
// While preclaim checks prevent clearing this flag when sfDomainID is set, we verify here
// to guard against potential logic regressions in the future changes.
if (view.rules().enabled(fixSecurity3_1_3))
{
// Post-fixSecurity3_1_3: Return a proper error code instead of asserting.
if (!(sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth))
return tefINTERNAL; // LCOV_EXCL_LINE
}
else
{
// Pre-fixSecurity3_1_3: Fault via assertion in Debug mode;
// potentially fall through in Release mode.
XRPL_ASSERT(
sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth,
"xrpl::requireAuth : issuance requires authorization");
}
// ter = tefINTERNAL | tecOBJECT_NOT_FOUND | tecNO_AUTH | tecEXPIRED
auto const ter = credentials::validDomain(view, *maybeDomainID, account);
if (isTesSuccess(ter))

View File

@@ -222,6 +222,11 @@ MPTokenIssuanceSet::preclaim(PreclaimContext const& ctx)
}))
return tecNO_PERMISSION;
// Clearing lsfMPTRequireAuth is invalid when the issuance already has
// a DomainID set, because a DomainID requires RequireAuth to be active.
if ((*mutableFlags & tmfMPTClearRequireAuth) != 0u &&
sleMptIssuance->isFieldPresent(sfDomainID))
return tecNO_PERMISSION;
if ((*mutableFlags & tmfMPTSetCanConfidentialAmount) ||
(*mutableFlags & tmfMPTClearCanConfidentialAmount))
{

View File

@@ -2997,45 +2997,83 @@ class MPToken_test : public beast::unit_test::suite
testcase("Mutate MPTRequireAuth");
using namespace test::jtx;
Env env{*this, features};
Account const alice("alice");
Account const bob("bob");
// test mutating RequireAuth flag on the issuance and its effect on payment authorization
{
Env env{*this, features};
Account const alice("alice");
Account const bob("bob");
MPTTester mptAlice(env, alice, {.holders = {bob}});
mptAlice.create(
{.ownerCount = 1,
.flags = tfMPTRequireAuth,
.mutableFlags = tmfMPTCanMutateRequireAuth});
MPTTester mptAlice(env, alice, {.holders = {bob}});
mptAlice.create(
{.ownerCount = 1,
.flags = tfMPTRequireAuth,
.mutableFlags = tmfMPTCanMutateRequireAuth});
mptAlice.authorize({.account = bob});
mptAlice.authorize({.account = alice, .holder = bob});
mptAlice.authorize({.account = bob});
mptAlice.authorize({.account = alice, .holder = bob});
// Pay to bob
mptAlice.pay(alice, bob, 1000);
// Pay to bob
mptAlice.pay(alice, bob, 1000);
// Unauthorize bob
mptAlice.authorize({.account = alice, .holder = bob, .flags = tfMPTUnauthorize});
// Unauthorize bob
mptAlice.authorize({.account = alice, .holder = bob, .flags = tfMPTUnauthorize});
// Can not pay to bob
mptAlice.pay(bob, alice, 100, tecNO_AUTH);
// Can not pay to bob
mptAlice.pay(bob, alice, 100, tecNO_AUTH);
// Clear RequireAuth
mptAlice.set({.account = alice, .mutableFlags = tmfMPTClearRequireAuth});
// Clear RequireAuth
mptAlice.set({.account = alice, .mutableFlags = tmfMPTClearRequireAuth});
// Can pay to bob
mptAlice.pay(alice, bob, 1000);
// Can pay to bob
mptAlice.pay(alice, bob, 1000);
// Set RequireAuth again
mptAlice.set({.account = alice, .mutableFlags = tmfMPTSetRequireAuth});
// Set RequireAuth again
mptAlice.set({.account = alice, .mutableFlags = tmfMPTSetRequireAuth});
// Can not pay to bob since he is not authorized
mptAlice.pay(bob, alice, 100, tecNO_AUTH);
// Can not pay to bob since he is not authorized
mptAlice.pay(bob, alice, 100, tecNO_AUTH);
// Authorize bob again
mptAlice.authorize({.account = alice, .holder = bob});
// Authorize bob again
mptAlice.authorize({.account = alice, .holder = bob});
// Can pay to bob again
mptAlice.pay(alice, bob, 100);
// Can pay to bob again
mptAlice.pay(alice, bob, 100);
}
// Cannot clear RequireAuth when a DomainID is set on the issuance
{
Account const alice{"alice"};
Account const bob{"bob"};
Account const credIssuer{"credIssuer"};
pdomain::Credentials const credentials{
{.issuer = credIssuer, .credType = "credential"}};
Env env{*this, features};
env.fund(XRP(1000), credIssuer);
env.close();
env(pdomain::setTx(credIssuer, credentials));
env.close();
auto const domainId = pdomain::getNewDomain(env.meta());
MPTTester mptAlice(env, alice, {.holders = {bob}});
mptAlice.create({
.ownerCount = 1,
.flags = tfMPTRequireAuth,
.mutableFlags = tmfMPTCanMutateRequireAuth,
.domainID = domainId,
});
// Clearing RequireAuth while a DomainID is present must be rejected,
mptAlice.set({
.account = alice,
.mutableFlags = tmfMPTClearRequireAuth,
.err = tecNO_PERMISSION,
});
// Setting RequireAuth (already set) is still allowed, though it has no effect.
mptAlice.set({.account = alice, .mutableFlags = tmfMPTSetRequireAuth});
}
}
void