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

Missing Deduction of Processed Fees in `FeeDistributionBranch::convertAccumulatedFeesToWeth` Function

0x23r0

Summary

In the convertAccumulatedFeesToWeth function, after swapping non-WETH assets to WETH, the contract fails to deduct the processed asset amount from the market's receivedFees mapping. This oversight allows the same fees to be converted multiple times, leading to potential double-spending or incorrect accounting of fees.

Vulnerability Details

The convertAccumulatedFeesToWeth function is responsible for converting collected collateral fees into WETH. After verifying that the market has received fees for the given asset and performing the swap, the function calls _handleWethRewardDistribution to distribute the WETH rewards. However, it does not remove or deduct the processed asset amount from the receivedFees mapping in the Market.Data struct. This means that the same fees could be processed multiple times, leading to incorrect accounting and potential exploitation.

Code Snippet:

function convertAccumulatedFeesToWeth(
uint128 marketId,
address asset,
uint128 dexSwapStrategyId,
bytes calldata path
)
external
onlyRegisteredSystemKeepers
{
// ...
​
// handles distribution of the weth reward between the protocol and market
_handleWethRewardDistribution(market, asset, ctx.receivedWethX18);
​
// Missing: market.receivedFees.remove(asset) or similar deduction
}

Impact

  • Double-Spending of Fees: The same fees can be converted multiple times, leading to an over-distribution of WETH rewards.

  • Incorrect Accounting: The receivedFees mapping will not accurately reflect the fees that have been processed, causing discrepancies in the contract's state.

Tools Used

  • Manual review

Recommendations

To fix this issue, the contract should deduct the processed asset amount from the receivedFees mapping after the fees have been successfully converted to WETH. This can be done by either removing the asset from the mapping or setting its value to zero.

function convertAccumulatedFeesToWeth(
uint128 marketId,
address asset,
uint128 dexSwapStrategyId,
bytes calldata path
)
external
onlyRegisteredSystemKeepers
{
// ...
​
// handles distribution of the weth reward between the protocol and market
_handleWethRewardDistribution(market, asset, ctx.receivedWethX18);
​
+ // Deduct the processed fees from the market's receivedFees mapping
+ market.receivedFees.remove(asset); // or market.receivedFees.set(asset, 0);
}

This change ensures that once fees are processed, they cannot be processed again, maintaining accurate accounting and preventing potential exploits.

Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!