Incorrect asset tracking in modifyExistingOrder
Description
For cross-asset pools, calling submit submitProducer(...)
always sets shouldFarm = false
and order.depositedShares > 0
using the oracle price. Orders are then enqueued with the pool asset:
if(shouldFarm) {
IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
order.depositedShares = IERC4626(vaultAdapter).deposit(amount, getAddressForPool(poolId)) / order.packetsRemaining;
} else {
IERC20(asset).safeTransferFrom(msg.sender, getAddressForPool(poolId), amount);
}
However, modifyExistingOrder(...)
has missing checks and assumes the orders were deposited in the vault asset:
if (order.depositedShares > 0) {
getWalletForPool(poolId).withdrawFromVault(amountTokens, msg.sender, vaultAdapters[pool.vault]);
} else {
getWalletForPool(poolId).withdraw(amountTokens, pool.asset, msg.sender);
}
Impact
An attacker could spam submitProducer(...)
and modifyExistingOrder(...)
to convert pool assets to vault assets at a rate of 1:1
. This could be financially lucrative as there are no ways to shut down the protocol or pull other users funds. It would also disrupt of the balance of the cross-asset pair and hence the potential operation of the pool.
Recommendations
Include logic to enure the vault and pool assets are correctly tracked in cross asset pools.
Remediation
Revest has implemented the following solution in committ 00000000000:
if (order.depositedShares > 0 && IERC4626(vaultAdapters[pool.vault]).asset() == pool.asset)
We find their remediation adequately addresses the concerns of this finding.