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

Fee recipient shares on Market Making Engine cannot be properly configured

h2134

Summary

Fee recipient share on Market Making Engine cannot be properly configured due to incorrect shares checking.

Vulnerability Details

Market Making Engine owner can call configureFeeRecipient() to configure fee share to an recipient.

This function checks if share is greater than zero to verify the total will not exceed the maximum shares, by comparing totalFeeRecipientsSharesX18 + share to Constants.MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES.

MarketMakingEngineConfigurationBranch::configureFeeRecipient():

// check if share is greater than zero to verify the total will not exceed the maximum shares
if (share > 0) {
UD60x18 totalFeeRecipientsSharesX18 = ud60x18(marketMakingEngineConfiguration.totalFeeRecipientsShares);
​
​
if (
@> totalFeeRecipientsSharesX18.add(ud60x18(share)).gt(
@> ud60x18(Constants.MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES)
)
) {
revert Errors.FeeRecipientShareExceedsLimit();
}
}

The checking is incorrect and may fail if the configured fee recipient has non-zero old share. Assuming:

  1. The Perp Engine is configured with 60% fee share, totalFeeRecipientsSharesX18 is 60%;

  2. Later owner tries to update the Perp Engine's fee share to 40% by calling **configureFeeRecipient() **and specifying share to 40%;

  3. Protocol compares totalFeeRecipientsSharesX18 + share to Constants.MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES which is 90%, because 60% + 40% > 90%, then transaction will be reverted and the new share cannot be configured.

Please run the coded POC in configureFeeRecipieint.t.sol:

function testAudit_FeeRecipientCannotBeConfigured() public {
// The max configuable protocol fee shares is 90%
assertEq(Constants.MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES, 0.9e18);
​
changePrank({ msgSender: users.owner.account });
​
// Set perpsEngine's fee share to 60%
marketMakingEngine.configureFeeRecipient(address(perpsEngine), 0.6e18);
​
// Update perpsEngine's fee share to 40%
vm.expectRevert(Errors.FeeRecipientShareExceedsLimit.selector);
marketMakingEngine.configureFeeRecipient(address(perpsEngine), 0.4e18);
}

Impact

Fee recipient cannot be configured.

Tools Used

Manual Review

Recommendations

Comparing totalFeeRecipientsSharesX18 + share - oldFeeRecipientShares to Constants.MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES:

// check if share is greater than zero to verify the total will not exceed the maximum shares
if (share > 0) {
UD60x18 totalFeeRecipientsSharesX18 = ud60x18(marketMakingEngineConfiguration.totalFeeRecipientsShares);
​
+ (, uint256 oldFeeRecipientShares) = marketMakingEngineConfiguration.protocolFeeRecipients.tryGet(feeRecipient);
​
if (
- totalFeeRecipientsSharesX18.add(ud60x18(share)).gt(
+ totalFeeRecipientsSharesX18.add(ud60x18(share)).sub(oldFeeRecipientShares).gt(
ud60x18(Constants.MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES)
)
) {
revert Errors.FeeRecipientShareExceedsLimit();
}
}
​
​
- (, uint256 oldFeeRecipientShares) = marketMakingEngineConfiguration.protocolFeeRecipients.tryGet(feeRecipient);
Updates

Lead Judging Commences

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

MarketMakingEngineConfigurationBranch::configureFeeRecipient can cause DOS when attempting to change share value of an existing address

Support

FAQs

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