Submission Details
Severity:
The function FeeDistributionBranch::_handleWethRewardDistribution miscalculates rewards as follows.
Summary
The function FeeDistributionBranch::_handleWethRewardDistribution miscalculates rewards as follows(test below).
path: /src/market-making/branches/FeeDistributionBranch/FeeDistributionBranch.sol
[FeeDistributionBranch::_handleWethRewardDistribution] logic assumes that the multiplication and subtraction will always produce the correct breakdown of rewards. If, for any misconfiguration (or due to rounding issues due to fixed-point math), the sum of receivedProtocolWethRewardX18 and receivedVaultsWethRewardX18 does not equal receivedWethX18, then the computed leftover might be wrong or lead to incorrect reward distributions.
// Calculate leftover reward (to cover any rounding error)UD60x18
leftover = receivedWethX18.sub(receivedProtocolWethRewardX18).sub(receivedVaultsWethRewardX18);
POC: foundry invaraint test below:
Assumptions:
If the invariant fails (i.e. if lastProtocolReward + lastVaultReward != lastReceivedWeth), then there is a math error in the reward splitting logic.
In this implementation the “leftover” is added to the vault reward to account for any rounding error in the fixed‑point math. The invariant ensures that the rounding “leftover” exactly makes up any difference so that all received WETH is fully distributed.
// SPDX-License-Identifier: MIT
import { UD60x18, ud60x18 } from "../../lib/prb-math/src/UD60x18.sol";
import "../../lib/prb-math/src/ud60x18/Helpers.sol" as Helpers; // /lib/prb-math/src/ud60x18/Helpers.sol
import { Constants } from "@zaros/utils/Constants.sol";
import { Math } from "@zaros/utils/Math.sol";
// For testing we import DSTest and Forge-std's Test.
import "forge-std/Test.sol";
/// @notice A dummy “market” contract that our _handleWethRewardDistribution function will use.
/// Instead of doing real bookkeeping, it just records the amounts passed in.
contract DummyMarket {
uint256 public protocolReward;
uint256 public vaultReward;
// This function is called by _handleWethRewardDistribution.
function receiveWethReward(
address /* assetOut */,
UD60x18 protocolRewardX18,
UD60x18 vaultRewardX18
) external {
// Unwrap the fixed-point numbers for ease of assertion in tests.
protocolReward = UD60x18.unwrap(protocolRewardX18);
vaultReward = UD60x18.unwrap(vaultRewardX18);
}
// Stub: getConnectedVaultsIds is required but not used in the math.
function getConnectedVaultsIds() external pure returns (uint256[] memory) {
return new uint256[](0);
}
}
/// @notice This contract contains the function under test. Because _handleWethRewardDistribution
/// is internal in production, we expose it via an external wrapper for testing purposes.
contract RewardDistributor {
// using UD60x18 for UD60x18;
// using Math for UD60x18;
// using Helpers for UD60x18;
/// @notice This wrapper lets us supply feeRecipientsSharesX18 as a parameter instead of reading
/// from configuration.
function handleWethRewardDistribution(
DummyMarket market,
address assetOut,
UD60x18 receivedWethX18,
UD60x18 feeRecipientsSharesX18
) public {
// Calculate the rewards as in the production function.
UD60x18 receivedProtocolWethRewardX18 = receivedWethX18.mul(feeRecipientsSharesX18);
UD60x18 receivedVaultsWethRewardX18 =
receivedWethX18.mul(ud60x18(Constants.MAX_SHARES).sub(feeRecipientsSharesX18));
// Calculate leftover reward (to cover any rounding error)
UD60x18 leftover = receivedWethX18.sub(receivedProtocolWethRewardX18).sub(receivedVaultsWethRewardX18);
// Add leftover to vault reward so that:
// protocolReward + vaultReward == receivedWethX18.
receivedVaultsWethRewardX18 = receivedVaultsWethRewardX18.add(leftover);
// Forward the rewards to the dummy market.
market.receiveWethReward(assetOut, receivedProtocolWethRewardX18, receivedVaultsWethRewardX18);
// (In production, the market would then recalculate vaults credit, etc.)
}
}
/// @notice The invariant fuzz test contract.
contract RewardDistributionInvariantTest is Test {
RewardDistributor public distributor;
DummyMarket public dummyMarket;
// We keep track of the most recent reward values so that we can assert the invariant.
uint256 public lastProtocolReward;
uint256 public lastVaultReward;
uint256 public lastReceivedWeth;
// An arbitrary asset address for testing.
address constant assetOut = address(0xBEEF);
function setUp() public {
distributor = new RewardDistributor();
dummyMarket = new DummyMarket();
}
/// @notice This function will be fuzzed with random fee recipient shares and received WETH amounts.
/// We bound feeShares to [0, Constants.MAX_SHARES] and receivedWeth to a reasonable range.
function testFuzz_handleWethRewardDistribution(uint256 feeSharesRaw, uint256 receivedWethRaw) public {
// Bound feeSharesRaw to the valid UD60x18 range: [0, MAX_SHARES].
// Constants.MAX_SHARES is 1e18 so we use that as our upper bound.
uint256 feeShares = bound(feeSharesRaw, 0, Constants.MAX_SHARES);
// Bound receivedWethRaw to a maximum value. (For example, up to 1e30 wei in fixed point.)
uint256 receivedWeth = bound(receivedWethRaw, 0, 1e30);
// Wrap values into UD60x18 fixed point numbers.
UD60x18 feeRecipientsSharesX18 = ud60x18(feeShares);
UD60x18 receivedWethX18 = ud60x18(receivedWeth);
// Call the reward distribution function.
distributor.handleWethRewardDistribution(dummyMarket, assetOut, receivedWethX18, feeRecipientsSharesX18);
// Record the results from the dummy market.
lastProtocolReward = dummyMarket.protocolReward();
lastVaultReward = dummyMarket.vaultReward();
lastReceivedWeth = receivedWeth;
}
/// @notice Invariant: The sum of the protocol reward and the vault reward (after adding any leftover)
/// must equal the original amount of received WETH.
function invariant_totalRewardIsDistributed() public {
uint256 totalDistributed = lastProtocolReward + lastVaultReward;
// The invariant must hold: distributed rewards equal the amount received.
assertEq(
totalDistributed,
lastReceivedWeth,
"Invariant failure: total distributed rewards do not equal received rewards"
);
}
}
Invaraint test output:
Failing tests:
Encountered 1 failing test in test/invariant/DummyMarket.t.sol:RewardDistributionInvariantTest
[FAIL: invariant_totalRewardIsDistributed persisted failure revert]
[Sequence]
sender=0x00000000000000000000000000000000000009AD addr=[test/invariant/DummyMarket.t.sol:RewardDistributor]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=handleWethRewardDistribution(address,address,uint256,uint256) args=[0x000000000000000000000000000000000A9254e3, 0x0000000000000000000000000000000000000043, 4639, 2939553533 [2.939e9]]
Ran 2 tests for test/invariant/DummyMarket.t.sol:RewardDistributionInvariantTest
[FAIL: invariant_totalRewardIsDistributed persisted failure revert]
[Sequence]
sender=0x00000000000000000000000000000000000009AD addr=[test/invariant/DummyMarket.t.sol:RewardDistributor]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=handleWethRewardDistribution(address,address,uint256,uint256) args=[0x000000000000000000000000000000000A9254e3, 0x0000000000000000000000000000000000000043, 4639, 2939553533 [2.939e9]]
invariant_totalRewardIsDistributed() (runs: 1, calls: 1, reverts: 1)
Impact
Mis-calculation of the rewards. I tagged this as high since this is Defi protocol and liquidity providers (LPs) would depend on correct calculations of fees and reward.
Tools Used
Manual review and foundry test
Recommendations
validate and normalize share values for example:
require(receivedProtocolWethRewardX18.add(receivedVaultsWethRewardX18) <= receivedWethX18, "Reward split error");
Updates
Appeal created
inallhonesty 9 months ago
Submission Judgement Published Assigned finding tags:
Rounding errors in _handleWethRewardDistribution cause overestimation of available rewards leading to protocol insolvency when users claim fees tag.
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.