mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-21 14:20:56 +00:00
fix: Add zero keylet check in credential (#7971)
This commit is contained in:
@@ -7,6 +7,7 @@
|
||||
#include <xrpl/ledger/ApplyView.h>
|
||||
#include <xrpl/ledger/ReadView.h>
|
||||
#include <xrpl/protocol/AccountID.h>
|
||||
#include <xrpl/protocol/Rules.h>
|
||||
#include <xrpl/protocol/STArray.h>
|
||||
#include <xrpl/protocol/STLedgerEntry.h>
|
||||
#include <xrpl/protocol/STTx.h>
|
||||
@@ -34,7 +35,7 @@ deleteSLE(ApplyView& view, SLE::ref sleCredential, beast::Journal j);
|
||||
|
||||
// Amendment and parameters checks for sfCredentialIDs field
|
||||
NotTEC
|
||||
checkFields(STTx const& tx, beast::Journal j);
|
||||
checkFields(STTx const& tx, Rules const& rules, beast::Journal j);
|
||||
|
||||
// Accessing the ledger to check if provided credentials are valid. Do not use
|
||||
// in doApply (only in preclaim) since it does not remove expired credentials.
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
#include <xrpl/protocol/TER.h>
|
||||
#include <xrpl/protocol/digest.h>
|
||||
|
||||
#include <algorithm>
|
||||
#include <cstdint>
|
||||
#include <expected>
|
||||
#include <limits>
|
||||
@@ -52,6 +53,9 @@ removeExpired(ApplyView& view, STVector256 const& arr, beast::Journal const j)
|
||||
for (auto const& h : arr)
|
||||
{
|
||||
// Credentials already checked in preclaim. Look only for expired here.
|
||||
if (view.rules().enabled(fixCleanup3_4_0) && h.isZero())
|
||||
return std::unexpected(tecINTERNAL); // LCOV_EXCL_LINE
|
||||
|
||||
auto const k = keylet::credential(h);
|
||||
auto const sleCred = view.peek(k);
|
||||
|
||||
@@ -124,7 +128,7 @@ deleteSLE(ApplyView& view, SLE::ref sleCredential, beast::Journal j)
|
||||
}
|
||||
|
||||
NotTEC
|
||||
checkFields(STTx const& tx, beast::Journal j)
|
||||
checkFields(STTx const& tx, Rules const& rules, beast::Journal j)
|
||||
{
|
||||
if (!tx.isFieldPresent(sfCredentialIDs))
|
||||
return tesSUCCESS;
|
||||
@@ -137,6 +141,13 @@ checkFields(STTx const& tx, beast::Journal j)
|
||||
return temMALFORMED;
|
||||
}
|
||||
|
||||
if (rules.enabled(fixCleanup3_4_0) &&
|
||||
std::ranges::any_of(credentials, [](uint256 const& id) { return id.isZero(); }))
|
||||
{
|
||||
JLOG(j.trace()) << "Malformed transaction: zero credential ID.";
|
||||
return temMALFORMED;
|
||||
}
|
||||
|
||||
std::unordered_set<uint256> duplicates;
|
||||
for (auto const& cred : credentials)
|
||||
{
|
||||
@@ -160,6 +171,14 @@ valid(STTx const& tx, ReadView const& view, AccountID const& src, beast::Journal
|
||||
auto const& credIDs(tx.getFieldV256(sfCredentialIDs));
|
||||
for (auto const& h : credIDs)
|
||||
{
|
||||
if (view.rules().enabled(fixCleanup3_4_0) && h.isZero())
|
||||
{
|
||||
// LCOV_EXCL_START
|
||||
JLOG(j.trace()) << "Zero credential ID.";
|
||||
return tecINTERNAL;
|
||||
// LCOV_EXCL_STOP
|
||||
}
|
||||
|
||||
auto const sleCred = view.read(keylet::credential(h));
|
||||
if (!sleCred)
|
||||
{
|
||||
@@ -234,6 +253,9 @@ authorizedDepositPreauth(ReadView const& view, STVector256 const& credIDs, Accou
|
||||
lifeExtender.reserve(credIDs.size());
|
||||
for (auto const& h : credIDs)
|
||||
{
|
||||
if (view.rules().enabled(fixCleanup3_4_0) && h.isZero())
|
||||
return tefINTERNAL; // LCOV_EXCL_LINE
|
||||
|
||||
auto sleCred = view.read(keylet::credential(h));
|
||||
if (!sleCred) // already checked in preclaim
|
||||
return tefINTERNAL; // LCOV_EXCL_LINE
|
||||
|
||||
@@ -50,7 +50,7 @@ AccountDelete::preflight(PreflightContext const& ctx)
|
||||
return temDST_IS_SRC;
|
||||
}
|
||||
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
|
||||
return err;
|
||||
|
||||
return tesSUCCESS;
|
||||
|
||||
@@ -111,7 +111,7 @@ EscrowFinish::preflightSigValidated(PreflightContext const& ctx)
|
||||
}
|
||||
}
|
||||
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
|
||||
return err;
|
||||
|
||||
return tesSUCCESS;
|
||||
|
||||
@@ -281,7 +281,7 @@ Payment::preflight(PreflightContext const& ctx)
|
||||
}
|
||||
}
|
||||
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
|
||||
return err;
|
||||
|
||||
return tesSUCCESS;
|
||||
|
||||
@@ -87,7 +87,7 @@ PaymentChannelClaim::preflight(PreflightContext const& ctx)
|
||||
return temBAD_SIGNATURE;
|
||||
}
|
||||
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
|
||||
return err;
|
||||
|
||||
return tesSUCCESS;
|
||||
|
||||
@@ -82,7 +82,7 @@ ConfidentialMPTSend::preflight(PreflightContext const& ctx)
|
||||
if (hasAuditor && !isValidCiphertext(ctx.tx[sfAuditorEncryptedAmount]))
|
||||
return temBAD_CIPHERTEXT;
|
||||
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
|
||||
if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
|
||||
return err;
|
||||
|
||||
return tesSUCCESS;
|
||||
|
||||
@@ -934,6 +934,46 @@ struct DepositPreauth_test : public beast::unit_test::Suite
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
testZeroCredentialID(FeatureBitset features)
|
||||
{
|
||||
testcase("Zero credential ID");
|
||||
|
||||
using namespace jtx;
|
||||
|
||||
char const credType[] = "abcde";
|
||||
Account const issuer{"issuer"};
|
||||
Account const alice{"alice"};
|
||||
Account const bob{"bob"};
|
||||
|
||||
Env env(*this, features);
|
||||
|
||||
env.fund(XRP(5000), issuer, alice, bob);
|
||||
env.close();
|
||||
|
||||
env(credentials::create(alice, issuer, credType));
|
||||
env.close();
|
||||
env(credentials::accept(alice, issuer, credType));
|
||||
env.close();
|
||||
|
||||
auto const jv = credentials::ledgerEntry(env, alice, issuer, credType);
|
||||
std::string const credIdx = jv[jss::result][jss::index].asString();
|
||||
|
||||
std::string const zeroIdx(64, '0');
|
||||
|
||||
// post-fixCleanup3_4_0: a zero ID is rejected by checkFields in
|
||||
// preflight; pre-fixCleanup3_4_0, it will trigger assertion, so it is not testable.
|
||||
env(pay(alice, bob, XRP(100)), credentials::Ids({zeroIdx}), Ter(temMALFORMED));
|
||||
env.close();
|
||||
|
||||
env(pay(alice, bob, XRP(100)), credentials::Ids({credIdx, zeroIdx}), Ter(temMALFORMED));
|
||||
env.close();
|
||||
|
||||
// A valid credential succeeds
|
||||
env(pay(alice, bob, XRP(100)), credentials::Ids({credIdx}));
|
||||
env.close();
|
||||
}
|
||||
|
||||
void
|
||||
testCredentialsCreation()
|
||||
{
|
||||
@@ -1446,6 +1486,7 @@ struct DepositPreauth_test : public beast::unit_test::Suite
|
||||
testPayment(supported - featureCredentials);
|
||||
testPayment(supported);
|
||||
testCredentialsPayment();
|
||||
testZeroCredentialID(supported);
|
||||
testCredentialsCreation();
|
||||
testExpiredCreds();
|
||||
testSortingCredentials();
|
||||
|
||||
Reference in New Issue
Block a user