A logical error in the share limit validation of configureFeeRecipient()
causes valid fee recipient configurations to be incorrectly rejected. The validation occurs before accounting for existing shares, leading to false maximum limit violations even when the final total would be within bounds.
MarketMakingEngineConfigurationBranch.sol:L613~654
The configureFeeRecipient()
function in MarketMakingEngineConfigurationBranch.sol
performs share limit validation before updating the total shares, which can lead to incorrect rejections of valid configurations. This can prevent legitimate fee recipient share adjustments even when the resulting total would be within allowed limits.
The current implementation validates the maximum share limit by adding the new share amount to the current total, without considering the existing share of the fee recipient being updated. This creates a logical error in the following sequence:
The function checks if currentTotal + newShare > MAX_LIMIT
Only after this check, it adjusts the total by accounting for the old shares
This order of operations means the validation doesn't accurately reflect the final state
For example, if:
Current total shares = 800
MAX_LIMIT = 1000
Existing recipient share = 300
New desired share = 400
The transaction would revert because 800 + 400 > 1000, even though the actual final total would be 900 (800 - 300 + 400), which is valid.
Move the share limit validation after the total shares calculation to ensure it accurately reflects the final state. The validation should occur after adjusting for any existing shares of the fee recipient.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.