Submission Details
Severity:
Incorrect Logic
## Summary
Incorrect logic in `MarketMakingEngineConfiguration::distributeProtocolAssetReward`.
## Vulnerability Details
The function `MarketMakingEngineConfiguration::distributeProtocolAssetReward` sends the accumulated protocol reward to the
configured recipients using the given asset.
However, inside the `for` loop if the `if()` block get executed the previous amount calculated(`feeRecipientReward`) in the
`for` loop for the last address in `feeRecipientsLength` will be overriden by whatever left in the contract. This will make
the last address receive zero(0) as `feeRecipientReward`for the scenario described below:
For example: `amountX18` = 1,000,000 (or 1,000,000 _ 1e18 to handle precision, in UD60x18 format). Total shares
(`totalFeeRecipientsSharesX18`) = 1,000,000 (or 1,000,000 _ 1e18). The protocol has 3 recipients with the following shares:
Recipient 1: 500,000 shares. Recipient 2: 300,000 shares. Recipient 3: 200,000 shares. Let's step through the calculations.
1. Initialize `amountX18` and `totalFeeRecipientsSharesX18`: `amountX18` = 1,000,000 _ 1e18 (representing the total amount to
distribute, in the proper UD60x18 format). `totalFeeRecipientsSharesX18` = 1,000,000 _ 1e18 (representing the total shares
of all recipients).
2. For Recipient 1 (shares = 500,000): The shares for recipient 1 are 500,000, and the total shares are 1,000,000. Calculate
their share of the reward:
`uint256 feeRecipientReward = amountX18.mul(ud60x18(500000)).div(totalFeeRecipientsSharesX18).intoUint256();`
`amountX18.mul(ud60x18(500000)) = 1,000,000 * 1e18 * 500,000 = 500,000 * 1e24.` Then, divide by
`totalFeeRecipientsSharesX18`:
`feeRecipientReward` = (500,000 _ 1e24) / (1,000,000 _ 1e18) = 500,000 \* 1e6 / 1,000,000 = 500,000. So, Recipient 1 will
receive 500,000.
3. For Recipient 2 (shares = 300,000):
`uint256 feeRecipientReward = amountX18.mul(ud60x18(300000)).div(totalFeeRecipientsSharesX18).intoUint256();`
`amountX18.mul(ud60x18(300000)) = 1,000,000 * 1e18 * 300,000 = 300,000 * 1e24.` Then, divide by
`totalFeeRecipientsSharesX18`:
`feeRecipientReward = (300,000 * 1e24) / (1,000,000 * 1e18) = 300,000 * 1e6 / 1,000,000 = 300,000.` So, Recipient 2 will
receive 300,000.
4. For Recipient 3 (shares = 200,000):
`uint256 feeRecipientReward = amountX18.mul(ud60x18(200000)).div(totalFeeRecipientsSharesX18).intoUint256();`
`amountX18.mul(ud60x18(200000)) = 1,000,000 * 1e18 * 200,000 = 200,000 * 1e24.` Then, divide by
totalFeeRecipientsSharesX18:
`feeRecipientReward = (200,000 * 1e24) / (1,000,000 * 1e18) = 200,000 * 1e6 / 1,000,000 = 200,000.` So, Recipient 3 will
receive 200,000.
The total is: 500,000 + 300,000 + 200,000 = 1,000,000
Therefore, `totalDistributed` is 1,000,000
But due to the `if()`, the last recipient(Recipient 3) will get 0 instead of 200,000.
5. In the `if()` block:
```javascript
if (i == feeRecipientsLength - 1) {
feeRecipientReward += amountX18.sub(ud60x18(totalDistributed)).intoUint256();
}
```
`amountX18` - `totalDistributed` = 1,000,000 - 1,000,000 = 0
## Impact
The last recipient gets 0 zero (0) as `feeRecipientReward` instead of `200,000`.
## Recommendations
Move the `if()` before `totalDistributed += feeRecipientReward;`
```diff
for (uint256 i; i < feeRecipientsLength; i++) {
// load the fee recipient address and shares
(address feeRecipient, uint256 shares) = self.protocolFeeRecipients.at(i);
// Calculate the fee recipient reward
uint256 feeRecipientReward = amountX18.mul(ud60x18(shares)).div(totalFeeRecipientsSharesX18).intoUint256();
+ // verify if is the last fee recipient
+ if (i == feeRecipientsLength - 1) {
+ feeRecipientReward += amountX18.sub(ud60x18(totalDistributed)).intoUint256();
+ }
// cache the total distributed
totalDistributed += feeRecipientReward;
- // verify if is the last fee recipient
- if (i == feeRecipientsLength - 1) {
- feeRecipientReward += amountX18.sub(ud60x18(totalDistributed)).intoUint256();
- }
// Transfer the fee recipient reward
IERC20(asset).safeTransfer(feeRecipient, feeRecipientReward);
}
```
Updates
Prize pool breakdown
Total prize
70,000 USDC
nSLOC
3,25821
USDC / LOC
66,500
3,500
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.