Part 2

Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Submission Details
Severity: medium
Invalid

Protocol fee shares can exceed 100% which is not intended

Summary

When updating an existing fee recipient's shares, there’s no validation to ensure the new total doesn’t exceed Constants.MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES (100%).

Vulnerability Details

function configureFeeRecipient(address feeRecipient, uint256 share) external onlyOwner {
// revert if protocolFeeRecipient is set to zero
if (feeRecipient == address(0)) revert Errors.ZeroInput("feeRecipient");
// load market making engine configuration data from storage
MarketMakingEngineConfiguration.Data storage marketMakingEngineConfiguration =
MarketMakingEngineConfiguration.load();
// 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();
}
}
(, uint256 oldFeeRecipientShares) = marketMakingEngineConfiguration.protocolFeeRecipients.tryGet(feeRecipient);
// update protocol total fee recipients shares value
if (oldFeeRecipientShares > 0) {
if (oldFeeRecipientShares > share) {
marketMakingEngineConfiguration.totalFeeRecipientsShares -=
(oldFeeRecipientShares - share).toUint128();
} else {
marketMakingEngineConfiguration.totalFeeRecipientsShares +=
(share - oldFeeRecipientShares).toUint128();
}
} else {
marketMakingEngineConfiguration.totalFeeRecipientsShares += share.toUint128();
}
// update protocol fee recipient
marketMakingEngineConfiguration.protocolFeeRecipients.set(feeRecipient, share);
// emit event LogConfigureFeeRecipient
emit LogConfigureFeeRecipient(feeRecipient, share);
}
/// @notice Updates the stability configuration settings, including the Chainlink verifier and maximum
/// verification delay.
/// @param chainlinkVerifier The address of the Chainlink verifier contract to be used for price verification.
/// @param maxVerificationDelay The maximum allowed delay, in seconds, for price verification.
function updateStabilityConfiguration(
address chainlinkVerifier,
uint128 maxVerificationDelay
)
external
onlyOwner
{
if (chainlinkVerifier == address(0)) {
revert Errors.ZeroInput("chainlinkVerifier");
}
if (maxVerificationDelay == 0) {
revert Errors.ZeroInput("maxVerificationDelay");
}
StabilityConfiguration.update(chainlinkVerifier, maxVerificationDelay);
emit LogUpdateStabilityConfig(chainlinkVerifier, maxVerificationDelay);
}

When updating an existing fee recipient's shares, there’s no validation to ensure the new total doesn’t exceed Constants.MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES (100%).

The if (share > 0) check only validates new shares (when oldShare == 0).

There is No validation when updating existing shares to prevent the totalFeeRecipientsShares from exceeding 100% (MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES).

Protocol fee shares can exceed 100%, breaking the invariant that total shares must never surpass 100%.
Example:

  • Initial total: 95% share.

  • Updating a recipient’s share from 5% to 10% increases the total to 100% (valid).

  • Updating a recipient’s share from 5% to 10% after another update could push the total to 105% (invalid, but unchecked).

Impact

Protocol fee shares can exceed 100%, breaking the invariant that total shares must never surpass 100%

Tools Used

Manual Review

Recommendations

// Add check AFTER updating total
UD60x18 newTotalShares = ud60x18(mmConfig.totalFeeRecipientsShares);
if (newTotalShares > ud60x18(Constants.MAX_CONFIGURABLE_PROTOCOL_FEE_SHARES)) {
revert Errors.FeeRecipientShareExceedsLimit();
}
mmConfig.protocolFeeRecipients.set(feeRecipient, share);
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 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.