ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-08-19 05:10:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

Enforce Destination checks on VaultWithdraw

Browse Source
This commit is contained in:
Bronek Kozicki
2025-04-01 13:39:26 +01:00
parent 93bd26547f
commit 2b86a1a557
2 changed files with 138 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

130
src/test/app/Vault_test.cpp
View File

@@ -68,6 +68,7 @@ class Vault_test : public beast::unit_test::suite
Account const& issuer,
Account const& owner,
Account const& depositor,
Account const& charlie,
Vault& vault,
PrettyAsset const& asset) {
auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
@@ -75,6 +76,15 @@ class Vault_test : public beast::unit_test::suite
env.close();
BEAST_EXPECT(env.le(keylet));
// Several 3rd party accounts which cannot receive funds
Account alice{"alice"};
Account dave{"dave"};
Account erin{"erin"}; // not authorized by issuer
env.fund(XRP(1000), alice, dave, erin);
env(fset(alice, asfDepositAuth));
env(fset(dave, asfRequireDest));
env.close();
{
testcase(prefix + " fail to set negative maximum");
auto tx = vault.set({.owner = owner, .id = keylet.key});
@@ -317,11 +327,57 @@ class Vault_test : public beast::unit_test::suite
}
{
testcase(prefix + " withdraw non-zero assets");
testcase(
prefix + " fail to withdraw to 3rd party lsfDepositAuth");
auto tx = vault.withdraw(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(200)});
.amount = asset(100)});
tx[sfDestination] = alice.human();
env(tx, ter{tecNO_PERMISSION});
}
if (!asset.raw().native())
{
testcase(
prefix + " fail to withdraw to 3rd party no authorization");
auto tx = vault.withdraw(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(100)});
tx[sfDestination] = erin.human();
env(tx,
ter{asset.raw().holds<Issue>() ? tecNO_LINE : tecNO_AUTH});
}
{
testcase(
prefix +
" fail to withdraw to 3rd party lsfRequireDestTag");
auto tx = vault.withdraw(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(100)});
tx[sfDestination] = dave.human();
env(tx, ter{tecDST_TAG_NEEDED});
}
{
testcase(prefix + " withdraw to authorized 3rd party");
auto tx = vault.withdraw(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(100)});
tx[sfDestination] = charlie.human();
env(tx);
}
{
testcase(prefix + " withdraw remaining assets");
auto tx = vault.withdraw(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(100)});
env(tx);
}
@@ -345,40 +401,57 @@ class Vault_test : public beast::unit_test::suite
}
};
auto testCases =
[this, &testSequence](
std::string prefix,
std::function<PrettyAsset(
Env & env, Account const& issuer, Account const& depositor)>
setup) {
Env env{*this};
Account issuer{"issuer"};
Account owner{"owner"};
Account depositor{"depositor"};
Vault vault{env};
env.fund(XRP(1000), issuer, owner, depositor);
env.close();
env(fset(issuer, asfAllowTrustLineClawback));
env.close();
env.require(flags(issuer, asfAllowTrustLineClawback));
auto testCases = [this, &testSequence](
std::string prefix,
std::function<PrettyAsset(
Env & env,
Account const& issuer,
Account const& owner,
Account const& depositor,
Account const& charlie)> setup) {
Env env{*this};
Account issuer{"issuer"};
Account owner{"owner"};
Account depositor{"depositor"};
Account charlie{"charlie"}; // authorized 3rd party
Vault vault{env};
env.fund(XRP(1000), issuer, owner, depositor, charlie);
env.close();
env(fset(issuer, asfAllowTrustLineClawback));
env(fset(issuer, asfRequireAuth));
env.close();
env.require(flags(issuer, asfAllowTrustLineClawback));
env.require(flags(issuer, asfRequireAuth));
PrettyAsset asset = setup(env, issuer, depositor);
testSequence(
prefix, env, issuer, owner, depositor, vault, asset);
};
PrettyAsset asset = setup(env, issuer, owner, depositor, charlie);
testSequence(
prefix, env, issuer, owner, depositor, charlie, vault, asset);
};
testCases(
"XRP",
[](Env& env, Account const& issuer, Account const& depositor)
-> PrettyAsset { return {xrpIssue(), 1'000'000}; });
[](Env& env,
Account const& issuer,
Account const& owner,
Account const& depositor,
Account const& charlie) -> PrettyAsset {
return {xrpIssue(), 1'000'000};
});
testCases(
"IOU",
[](Env& env,
Account const& issuer,
Account const& depositor) -> Asset {
Account const& owner,
Account const& depositor,
Account const& charlie) -> Asset {
PrettyAsset asset = issuer["IOU"];
env.trust(asset(1000), depositor);
env(trust(owner, asset(1000)));
env(trust(depositor, asset(1000)));
env(trust(charlie, asset(1000)));
env(trust(issuer, asset(0), owner, tfSetfAuth));
env(trust(issuer, asset(0), depositor, tfSetfAuth));
env(trust(issuer, asset(0), charlie, tfSetfAuth));
env(pay(issuer, depositor, asset(1000)));
env.close();
return asset;
@@ -388,13 +461,16 @@ class Vault_test : public beast::unit_test::suite
"MPT",
[](Env& env,
Account const& issuer,
Account const& depositor) -> Asset {
Account const& owner,
Account const& depositor,
Account const& charlie) -> Asset {
MPTTester mptt{env, issuer, mptInitNoFund};
mptt.create(
{.flags =
tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock});
PrettyAsset asset = mptt.issuanceID();
mptt.authorize({.account = depositor});
mptt.authorize({.account = charlie});
env(pay(issuer, depositor, asset(1000)));
env.close();
return asset;

54
src/xrpld/app/tx/detail/VaultWithdraw.cpp
View File

@@ -65,24 +65,16 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
if (!vault)
return tecNO_ENTRY;
if (ctx.tx.isFieldPresent(sfDestination))
{
auto const dstAccountID = ctx.tx.getAccountID(sfDestination);
if (auto const sleDst = ctx.view.read(keylet::account(dstAccountID));
sleDst == nullptr)
return tecNO_DST;
}
// Enforce valid withdrawal policy
if (vault->at(sfWithdrawalPolicy) != vaultStrategyFirstComeFirstServe)
return tefINTERNAL;
auto const assets = ctx.tx[sfAmount];
auto const asset = vault->at(sfAsset);
auto const share = vault->at(sfShareMPTID);
if (assets.asset() != asset && assets.asset() != share)
return tecWRONG_ASSET;
// Enforce valid withdrawal policy
if (vault->at(sfWithdrawalPolicy) != vaultStrategyFirstComeFirstServe)
return tefINTERNAL;
auto const account = ctx.tx[sfAccount];
auto const dstAcct = [&]() -> AccountID {
if (ctx.tx.isFieldPresent(sfDestination))
@@ -90,14 +82,38 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
return account;
}();
if (account != dstAcct && assets.holds<MPTIssue>())
// Withdrawal to a 3rd party destination account is essentially a transfer,
// via shares in the vault. Enforce all the usual asset transfer checks.
if (account != dstAcct)
{
auto mptID = assets.get<MPTIssue>().getMptID();
auto issuance = ctx.view.read(keylet::mptIssuance(mptID));
if (!issuance)
return tecOBJECT_NOT_FOUND;
if ((issuance->getFlags() & lsfMPTCanTransfer) == 0)
return tecNO_AUTH;
auto const sleDst = ctx.view.read(keylet::account(dstAcct));
if (sleDst == nullptr)
return tecNO_DST;
if (sleDst->getFlags() & lsfRequireDestTag)
return tecDST_TAG_NEEDED; // Cannot send without a tag
if (sleDst->getFlags() & lsfDepositAuth)
{
if (!ctx.view.exists(keylet::depositPreauth(dstAcct, account)))
return tecNO_PERMISSION;
}
if (auto const ter = std::visit(
[&]<ValidIssueType TIss>(TIss const& issue) -> TER {
return requireAuth(ctx.view, issue, dstAcct);
},
asset.value());
!isTesSuccess(ter))
return ter;
if (assets.holds<MPTIssue>())
{
if (auto const ter = canTransfer(
ctx.view, assets.get<MPTIssue>(), account, dstAcct);
!isTesSuccess(ter))
return ter;
}
}
// Cannot withdraw from a Vault an Asset frozen for the destination account