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

No Mechanism to Remove Fee Recipients Can Lead to Failed Reward Distributions

0xdarko

Summary

The protocol lacks functionality to remove fee recipients from protocolFeeRecipients. The only way to "disable" a recipient is by setting their share to zero, but they remain in the Enumerable Mapping and still receive transfer calls. This can cause entire reward distributions to fail if any recipient becomes invalid

Vulnerability Details

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/35deb3e92b2a32cd304bf61d27e6071ef36e446d/src/market-making/branches/MarketMakingEngineConfigurationBranch.sol#L613C1-L654C6

In configureFeeRecipient, recipients can only have their shares modified:

​
function configureFeeRecipient(address feeRecipient, uint256 share) external onlyOwner {
// Can set share to 0 but recipient remains in array
marketMakingEngineConfiguration.protocolFeeRecipients.set(feeRecipient, share);
}
​

During reward distribution, transfers are attempted to all recipients regardless of share amount:

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/35deb3e92b2a32cd304bf61d27e6071ef36e446d/src/market-making/leaves/MarketMakingEngineConfiguration.sol#L69C1-L87C74

function distributeProtocolAssetReward(Data storage self, address asset, uint256 amount) internal {
for (uint256 i; i < feeRecipientsLength; i++) {
(address feeRecipient, uint256 shares) = self.protocolFeeRecipients.at(i);
uint256 feeRecipientReward = amountX18.mul(ud60x18(shares))
.div(totalFeeRecipientsSharesX18).intoUint256();
// Transfer attempted even if shares or reward is 0
// Will revert if recipient can't receive tokens
IERC20(asset).safeTransfer(feeRecipient, feeRecipientReward);
}
}

This is called in critical protocol functions:

  • fulfillSwap: Distributes swap fees

  • _convertAssetsToUsdc: Handles USDC conversions

  • sendWethToFeeRecipients: Distributes WETH rewards

Impact

  • If any recipient becomes invalid (blacklisted, reverts on transfer, etc.) or receives a zero-value transfer with a token that doesn't allow it and reverts, all reward distributions will fail

  • No way to remove problematic recipients

  • Gas inefficiency from iterating over and attempting transfers to zero-share recipients

  • Could completely block core protocol functions that depend on successful fee distribution

Tools Used

Foundry

Recommendations

Inside configureFeeRecipient if shares == 0 add an if statement at the end of updating protocol total fee shares values

function configureFeeRecipient(address feeRecipient, uint256 share) external onlyOwner {
// ... share checks ...
// First update total shares
if (oldFeeRecipientShares > 0) {
// ... share calculations ...
}
​
// Then handle recipient
+ if(share == 0) {
+ marketMakingEngineConfiguration.protocolFeeRecipients.remove(feeRecipient);
+ } else {
marketMakingEngineConfiguration.protocolFeeRecipients.set(feeRecipient, share);
+ }
}
Updates

Lead Judging Commences

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

A blacklisted fee recipient will DoS the distributeProtocolAssetReward function because there's no way of removing them from array.

Support

FAQs

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