Root + Impact
Description
In normal operation, the contract allows the owner to add new ERC20 tokens that sellers can list via the setAllowedSellToken() function. It also provides an emergencyWithdrawERC20() function that allows the owner to withdraw arbitrary ERC20 tokens from the contract — with the exception of a hardcoded list of "core" tokens.
The issue is that the contract does not protect against owner abuse of dynamically added tokens. This allows a malicious owner to:
List a malicious or deceptive token.
Wait for users to deposit it into the system.
Withdraw all user deposits via emergencyWithdrawERC20.
function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {
if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken();
@> allowedSellToken[_token] = _isAllowed;
}
function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
if (
@> _tokenAddress == address(iWETH) || _tokenAddress == address(iWBTC) || _tokenAddress == address(iWSOL)
|| _tokenAddress == address(iUSDC)
) {
revert("Cannot withdraw core order book tokens via emergency function");
}
@>
IERC20 token = IERC20(_tokenAddress);
token.safeTransfer(_to, _amount);
}
Risk
Likelihood:
The admin can call setAllowedSellToken() at any time and approve a new token that users can start using.
The admin can call emergencyWithdrawERC20() at any time and withdraw user-deposited tokens unless the token is hardcoded as protected.
Impact:
Proof of Concept
function testMaliciousTokenRugPull() public {
vm.startPrank(owner);
MockUSDT malicious = new MockUSDT(6);
malicious.mint(alice, 1e6);
book.setAllowedSellToken(address(malicious), true);
vm.startPrank(alice);
malicious.approve(address(book), 1e6);
book.createSellOrder(address(malicious), 1, 10_000_000, block.timestamp);
vm.stopPrank();
vm.prank(owner);
book.emergencyWithdrawERC20(address(malicious), 1, owner);
assertEq(malicious.balanceOf(owner), 1, "owner have user funds");
}
Recommended Mitigation
- function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
- if (
- _tokenAddress == address(iWETH) || _tokenAddress == address(iWBTC) || _tokenAddress == address(iWSOL)
- || _tokenAddress == address(iUSDC)
- ) {
- revert("Cannot withdraw core order book tokens via emergency function");
- }
+ function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
+ if (allowedSellToken[_tokenAddress]) {
+ revert("Cannot emergency withdraw allowed sell token");
+ }
This change prevents the admin from using emergencyWithdrawERC20() to rug tokens that were previously approved for user deposits via setAllowedSellToken.