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

Missing Check for Token Approval in `FeeDistributionBranch::receiveMarketFee`

auditbyte

Vulnerability Details

The function receiveMarketFee calls IERC20(asset).safeTransferFrom(msg.sender, address(this), amount); to transfer tokens from msg.sender to the contract. However, it does not check if the sender has approved enough tokens before making the transfer call. If the sender has not approved the required amount, the transaction will revert, which could disrupt the intended operation.

function receiveMarketFee(
uint128 marketId,
address asset,
uint256 amount
)
external
onlyRegisteredEngine(marketId)
{
// verify input amount
if (amount == 0) revert Errors.ZeroInput("amount");
​
// loads the market data storage pointer
Market.Data storage market = Market.loadExisting(marketId);
​
// loads the collateral's data storage pointer
Collateral.Data storage collateral = Collateral.load(asset);
​
// reverts if collateral isn't supported
collateral.verifyIsEnabled();
​
// convert uint256 -> UD60x18; scales input amount to 18 decimals
UD60x18 amountX18 = collateral.convertTokenAmountToUd60x18(amount);
​
// transfer fee amount
// note: we're calling `ERC20::transferFrom` before state transitions as `_handleWethRewardDistribution`
// requires assets to be in the contract
IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
​
if (asset == MarketMakingEngineConfiguration.load().weth) {
// if asset is weth, we can skip straight to handling the weth reward distribution
_handleWethRewardDistribution(market, address(0), amountX18);
} else {
// update [asset => received fees] mapping
market.depositFee(asset, amountX18);
}
​
// emit event to log the received fee
emit LogReceiveMarketFee(asset, marketId, amount);
}

Impact

If the msg.sender has not approved enough tokens, the transaction will fail with a standard ERC-20 "transfer amount exceeds allowance" error.
Users may not understand why their transaction failed since the function does not explicitly check and revert with a clear error message.
Since the approval check is not done before calling safeTransferFrom, if the transaction fails, the sender still loses gas fees.
In a critical function repeated failures could disrupt operations, causing a Denial of Service (DoS).

Recommendations

It is advisable to add explicit validation using IERC20(asset).allowance(msg.sender, address(this)).

Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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