mirror of
https://github.com/XRPLF/rippled.git
synced 2026-03-30 16:42:30 +00:00
fix: Check trustline limits for share-denominated vault withdrawals
When a VaultWithdraw specifies the amount in shares (MPT) rather than assets, convert to the equivalent asset amount before calling canWithdraw. Previously the limit check was skipped for share-denominated withdrawals, allowing funds to exceed the destination's trustline limit.
This commit is contained in:
@@ -41,10 +41,20 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
|
||||
if (!vault)
|
||||
return tecNO_ENTRY;
|
||||
|
||||
auto const assets = ctx.tx[sfAmount];
|
||||
auto const mptIssuanceID = vault->at(sfShareMPTID);
|
||||
auto const sleIssuance = ctx.view.read(keylet::mptIssuance(mptIssuanceID));
|
||||
if (!sleIssuance)
|
||||
{
|
||||
// LCOV_EXCL_START
|
||||
JLOG(ctx.j.error()) << "VaultWithdraw: missing issuance of vault shares.";
|
||||
return tefINTERNAL;
|
||||
// LCOV_EXCL_STOP
|
||||
}
|
||||
|
||||
auto const amount = ctx.tx[sfAmount];
|
||||
auto const vaultAsset = vault->at(sfAsset);
|
||||
auto const vaultShare = vault->at(sfShareMPTID);
|
||||
if (assets.asset() != vaultAsset && assets.asset() != vaultShare)
|
||||
if (amount.asset() != vaultAsset && amount.asset() != vaultShare)
|
||||
return tecWRONG_ASSET;
|
||||
|
||||
auto const& vaultAccount = vault->at(sfAccount);
|
||||
@@ -65,8 +75,26 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
|
||||
// LCOV_EXCL_STOP
|
||||
}
|
||||
|
||||
if (auto const ret = canWithdraw(ctx.view, ctx.tx))
|
||||
return ret;
|
||||
if (amount.asset() == vaultShare)
|
||||
{
|
||||
// If the user specified shares, we need to first convert them to asset amount before
|
||||
// checking whether they can be withdrawn
|
||||
auto const maybeAssets = sharesToAssetsWithdraw(vault, sleIssuance, amount);
|
||||
if (!maybeAssets)
|
||||
return tecINTERNAL; // LCOV_EXCL_LINE
|
||||
|
||||
auto const from = ctx.tx[sfAccount];
|
||||
auto const to = ctx.tx[~sfDestination].value_or(from);
|
||||
|
||||
if (auto const ret = canWithdraw(
|
||||
ctx.view, from, to, *maybeAssets, ctx.tx.isFieldPresent(sfDestinationTag)))
|
||||
return ret;
|
||||
}
|
||||
else
|
||||
{
|
||||
if (auto const ret = canWithdraw(ctx.view, ctx.tx))
|
||||
return ret;
|
||||
}
|
||||
|
||||
// If sending to Account (i.e. not a transfer), we will also create (only
|
||||
// if authorized) a trust line or MPToken as needed, in doApply().
|
||||
|
||||
@@ -5350,6 +5350,79 @@ class Vault_test : public beast::unit_test::suite
|
||||
}
|
||||
}
|
||||
|
||||
// Reproduction: canWithdraw IOU limit check bypassed when
|
||||
// withdrawal amount is specified in shares (MPT) rather than in assets.
|
||||
void
|
||||
testBug6_LimitBypassWithShares()
|
||||
{
|
||||
using namespace test::jtx;
|
||||
testcase("Bug6 - limit bypass with share-denominated withdrawal");
|
||||
|
||||
Env env{*this, testable_amendments() | featureSingleAssetVault};
|
||||
Account const owner{"owner"};
|
||||
Account const issuer{"issuer"};
|
||||
Account const depositor{"depositor"};
|
||||
Account const charlie{"charlie"};
|
||||
Vault vault{env};
|
||||
|
||||
env.fund(XRP(1000), issuer, owner, depositor, charlie);
|
||||
env(fset(issuer, asfAllowTrustLineClawback));
|
||||
env.close();
|
||||
|
||||
PrettyAsset const asset = issuer["IOU"];
|
||||
env.trust(asset(1000), owner);
|
||||
env.trust(asset(1000), depositor);
|
||||
env(pay(issuer, owner, asset(200)));
|
||||
env(pay(issuer, depositor, asset(200)));
|
||||
env.close();
|
||||
|
||||
// Charlie gets a LOW trustline limit of 5
|
||||
env.trust(asset(5), charlie);
|
||||
env.close();
|
||||
|
||||
auto const [tx, keylet] = vault.create({.owner = owner, .asset = asset});
|
||||
env(tx);
|
||||
env.close();
|
||||
|
||||
auto const depositTx =
|
||||
vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)});
|
||||
env(depositTx);
|
||||
env.close();
|
||||
|
||||
// Get the share MPT info
|
||||
auto const vaultSle = env.le(keylet);
|
||||
if (!BEAST_EXPECT(vaultSle))
|
||||
return;
|
||||
auto const mptIssuanceID = vaultSle->at(sfShareMPTID);
|
||||
MPTIssue const shares(mptIssuanceID);
|
||||
PrettyAsset const share(shares);
|
||||
|
||||
// CONTROL: Withdraw 10 IOU (asset-denominated) to charlie.
|
||||
// Charlie's limit is 5, so this should be rejected with tecNO_LINE.
|
||||
{
|
||||
auto withdrawTx =
|
||||
vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = asset(10)});
|
||||
withdrawTx[sfDestination] = charlie.human();
|
||||
env(withdrawTx, ter{tecNO_LINE});
|
||||
env.close();
|
||||
}
|
||||
auto const charlieBalanceBefore = env.balance(charlie, asset.raw().get<Issue>());
|
||||
|
||||
// Withdraw the equivalent amount in shares to charlie. This should also be rejected.<
|
||||
{
|
||||
auto withdrawTx = vault.withdraw(
|
||||
{.depositor = depositor, .id = keylet.key, .amount = STAmount(share, 10'000'000)});
|
||||
withdrawTx[sfDestination] = charlie.human();
|
||||
env(withdrawTx, ter{tecNO_LINE});
|
||||
env.close();
|
||||
|
||||
// Verify that charlie received IOU beyond their trustline limit
|
||||
// (their limit is 5, but they now hold 10).
|
||||
auto const charlieBalanceAfter = env.balance(charlie, asset.raw().get<Issue>());
|
||||
BEAST_EXPECT(charlieBalanceAfter == charlieBalanceBefore);
|
||||
}
|
||||
}
|
||||
|
||||
public:
|
||||
void
|
||||
run() override
|
||||
@@ -5370,6 +5443,7 @@ public:
|
||||
testVaultClawbackBurnShares();
|
||||
testVaultClawbackAssets();
|
||||
testAssetsMaximum();
|
||||
testBug6_LimitBypassWithShares();
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user