Summary
Following Weird ERC20s are not supported
Vulnerability details
Some weird ERC20s are not supported in the protocol. Following are the list of them:
NOTE:
Test given in the file are written in Foundry not hardhat. To setup and run the test, read and follow the instruction given in readme of this github repo.
Also all of the test below are already given in the repo.
1. Zero Approval Tokens
Some ERC20 tokens only let an address approve to other address if the previous approval is zero. But this is not always the case in the LiquidationPoolManager. If sometimes the Liquidated assets becomes more than the staked balances then their will be some approval left for LiquidationPool by LiquidationPoolManger when LiquidationPool::distributeAssets(...) is called.
Also in LiquidationPool::distributeAssets(...), there is division before multiplication that can cause rounding errors. So it can leave dust approvals and can cause issues. If an active approval is left then next time an asset is Liquidated, it will not be possible to distribute those to stakers.
Here is a test for PoC
function test_zeroApprovalERC20NotSupported() public {
uint tstAmount = 1000 ether;
uint eurosAmount = 1000 ether;
tokens.eurosToken.mint(bob, eurosAmount);
tokens.tstToken.mint(bob, tstAmount);
tokens.eurosToken.mint(alice, eurosAmount);
tokens.tstToken.mint(alice, tstAmount);
vm.startPrank(bob);
tokens.eurosToken.approve(address(contracts.liquidationPool), eurosAmount);
tokens.tstToken.approve(address(contracts.liquidationPool), tstAmount);
contracts.liquidationPool.increasePosition(tstAmount, eurosAmount);
vm.stopPrank();
vm.startPrank(alice);
tokens.eurosToken.approve(address(contracts.liquidationPool), eurosAmount);
tokens.tstToken.approve(address(contracts.liquidationPool), tstAmount);
contracts.liquidationPool.increasePosition(tstAmount, eurosAmount);
vm.stopPrank();
skip(block.timestamp + 1 weeks);
ZeroApprovalToken zeroApprovalToken = new ZeroApprovalToken(100000 ether);
contracts.tokenManager.addAcceptedToken(address(zeroApprovalToken), address(priceFeeds.arbUsdPriceFeed));
zeroApprovalToken.mint(address(contracts.liquidationPoolManager), 10 ether + 133 wei);
vm.prank(address(contracts.liquidationPoolManager));
zeroApprovalToken.approve(address(contracts.liquidationPool), 10 ether + 133 wei);
ILiquidationPoolManager.Asset[1] memory _assets;
uint256 rewardsBalance = 10 ether + 133 wei;
_assets[0] = ILiquidationPoolManager.Asset(
ITokenManager.Token({
symbol: 'TKN',
addr: address(zeroApprovalToken),
dec: 18,
clAddr: address(priceFeeds.arbUsdPriceFeed),
clDec: 8
}),
rewardsBalance
);
assets.push(_assets[0]);
vm.startPrank(address(contracts.liquidationPoolManager));
contracts.liquidationPool.distributeAssets(assets, uint(constants.DEFAULT_COLLATERAL_RATE), 100_000);
vm.stopPrank();
zeroApprovalToken.mint(address(contracts.liquidationPoolManager), 1 ether);
vm.prank(address(contracts.liquidationPoolManager));
vm.expectRevert("unsafe-approve");
zeroApprovalToken.approve(address(contracts.liquidationPool), 1 ether);
}
Output:
Running 1 test for test/unit/Unit.t.sol:Unit
[PASS] test_zeroApprovalERC20NotSupported() (gas: 1295547)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.80ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
2. Rebasing Tokens
ERC20 rebasing tokens should not be added as a collateral token as their total supply is adjusted for inflation and deflation. The wallet having that has these token will see change in their balance becuase of rebasing. Because of this, these rebasing tokens uses concept of shares and also seperate function are used to calculate the shares to asset conversion and transferring shares etc. and that is not at all supported in the system anywhere.
3. BlockList tokens
Some tokens follows the concept of blocklist. Basically they can block an address so that it cannot mint ,transfer , receive etc any the token. If this type of tokens is added as a collateral than the staker who is already in the block list of the token, will not be able to claim any of his rewards. Also if the user is blocked later by the Token after he created his vault, he will not be able to withdraw his tokens from the vault.
Here is a test for PoC:
function test_blockListTokensWillCauseDistributionAssetIssue() public {
uint tstAmount = 1000 ether;
uint eurosAmount = 1000 ether;
tokens.eurosToken.mint(bob, eurosAmount);
tokens.tstToken.mint(bob, tstAmount);
tokens.eurosToken.mint(alice, eurosAmount);
tokens.tstToken.mint(alice, tstAmount);
vm.startPrank(bob);
tokens.eurosToken.approve(address(contracts.liquidationPool), eurosAmount);
tokens.tstToken.approve(address(contracts.liquidationPool), tstAmount);
contracts.liquidationPool.increasePosition(tstAmount, eurosAmount);
vm.stopPrank();
vm.startPrank(alice);
tokens.eurosToken.approve(address(contracts.liquidationPool), eurosAmount);
tokens.tstToken.approve(address(contracts.liquidationPool), tstAmount);
contracts.liquidationPool.increasePosition(tstAmount, eurosAmount);
vm.stopPrank();
skip(block.timestamp + 1 weeks);
BlockListToken blockListToken = new BlockListToken(100000 ether);
contracts.tokenManager.addAcceptedToken(address(blockListToken), address(priceFeeds.arbUsdPriceFeed));
blockListToken.mint(address(contracts.liquidationPoolManager), 10 ether );
vm.prank(address(contracts.liquidationPoolManager));
blockListToken.approve(address(contracts.liquidationPool), 10 ether);
ILiquidationPoolManager.Asset[1] memory _assets;
uint256 rewardsBalance = 10 ether;
_assets[0] = ILiquidationPoolManager.Asset(
ITokenManager.Token({
symbol: 'TKN',
addr: address(blockListToken),
dec: 18,
clAddr: address(priceFeeds.arbUsdPriceFeed),
clDec: 8
}),
rewardsBalance
);
assets.push(_assets[0]);
blockListToken.block(alice);
vm.startPrank(address(contracts.liquidationPoolManager));
contracts.liquidationPool.distributeAssets(assets, uint(constants.DEFAULT_COLLATERAL_RATE), 100_000);
vm.stopPrank();
vm.startPrank(alice);
vm.expectRevert("blocked");
contracts.liquidationPool.claimRewards();
vm.stopPrank();
}
Output:
Running 1 test for test/unit/Unit.t.sol:Unit
[PASS] test_blockListTokensWillCauseDistributionAssetIssue() (gas: 1485760)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.85ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
4. Fee on Transfer Token
Some tokens has a concept of fee on Transfers. When an address tries to transfer the tokens, it deducts some amount for it as a fee for doing the transfer. If this type of token is used then the user will not be able to claim his tokens as the number of tokens added to his rewards will be less than the amount of token LiquidationPool will hold as the transfer of tokens is after the tokens are added to the rewards balance of the user.
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
...
_position.EUROs -= costInEuros;
@> rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
burnEuros += costInEuros;
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
@> IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}
}
...
}
GitHub: 205-241
Here is a test that proves that:
function test_FeeOnTransferTokensNotSupported() public {
uint tstAmount = 1000 ether;
uint eurosAmount = 1000 ether;
tokens.eurosToken.mint(alice, eurosAmount);
tokens.tstToken.mint(alice, tstAmount);
vm.startPrank(alice);
tokens.eurosToken.approve(address(contracts.liquidationPool), eurosAmount);
tokens.tstToken.approve(address(contracts.liquidationPool), tstAmount);
contracts.liquidationPool.increasePosition(tstAmount, eurosAmount);
vm.stopPrank();
skip(block.timestamp + 1 weeks);
FeeOnTransferToken feeOnTransferToken = new FeeOnTransferToken(100000 ether);
contracts.tokenManager.addAcceptedToken(address(feeOnTransferToken), address(priceFeeds.arbUsdPriceFeed));
feeOnTransferToken.mint(address(contracts.liquidationPoolManager), 10 ether );
vm.prank(address(contracts.liquidationPoolManager));
feeOnTransferToken.approve(address(contracts.liquidationPool), 10 ether);
ILiquidationPoolManager.Asset[1] memory _assets;
uint256 rewardsBalance = 10 ether;
_assets[0] = ILiquidationPoolManager.Asset(
ITokenManager.Token({
symbol: 'TKN',
addr: address(feeOnTransferToken),
dec: 18,
clAddr: address(priceFeeds.arbUsdPriceFeed),
clDec: 8
}),
rewardsBalance
);
assets.push(_assets[0]);
vm.startPrank(address(contracts.liquidationPoolManager));
contracts.liquidationPool.distributeAssets(assets, uint(constants.DEFAULT_COLLATERAL_RATE), 100_000);
vm.stopPrank();
vm.startPrank(alice);
vm.expectRevert("insufficient-balance");
contracts.liquidationPool.claimRewards();
vm.stopPrank();
}
Output:
├─ [46352] LiquidationPool::claimRewards()
│ ├─ [31150] TokenManager::getAcceptedTokens() [staticcall]
│ │ └─ ← [Token({ symbol: 0x4554480000000000000000000000000000000000000000000000000000000000, addr: 0x0000000000000000000000000000000000000000, dec: 18, clAddr: 0xD6BbDE9174b1CdAa358d2Cf4D57D1a9F7178FBfF, clDec: 8 }), Token({ symbol: 0x5742544300000000000000000000000000000000000000000000000000000000, addr: 0xF62849F9A0B5Bf2913b396098F7c7019b51A820a, dec: 8, clAddr: 0x3D7Ebc40AF7092E3F1C81F2e996cbA5Cae2090d7, clDec: 8 }), Token({ symbol: 0x4152420000000000000000000000000000000000000000000000000000000000, addr: 0xc7183455a4C133Ae270771860664b6B7ec320bB1, dec: 18, clAddr: 0x2a07706473244BC757E10F2a9E86fB532828afe3, clDec: 8 }), Token({ symbol: 0x4c494e4b00000000000000000000000000000000000000000000000000000000, addr: 0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9, dec: 18, clAddr: 0x212224D2F2d262cd093eE13240ca4873fcCBbA3C, clDec: 8 }), Token({ symbol: 0x5041584700000000000000000000000000000000000000000000000000000000, addr: 0xa0Cb889707d426A7A386870A03bc70d1b0697598, dec: 18, clAddr: 0x15cF58144EF33af1e14b5208015d11F9143E27b9, clDec: 8 }), Token({ symbol: 0x5745544800000000000000000000000000000000000000000000000000000000, addr: 0xA4AD4f68d0b91CFD19687c881e50f3A00242828c, dec: 18, clAddr: 0xD6BbDE9174b1CdAa358d2Cf4D57D1a9F7178FBfF, clDec: 8 }), Token({ symbol: 0x544b4e0000000000000000000000000000000000000000000000000000000000, addr: 0x796f2974e3C1af763252512dd6d521E9E984726C, dec: 18, clAddr: 0x2a07706473244BC757E10F2a9E86fB532828afe3, clDec: 8 })]
│ ├─ [730] FeeOnTransferToken::transfer(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], 10000000000000000000 [1e19])
│ │ └─ ← revert: insufficient-balance
│ └─ ← revert: insufficient-balance
├─ [0] VM::stopPrank()
│ └─ ← ()
└─ ← ()
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.27ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
5. Non Standard ERC20
Some ERC20 tokens doesn't follow the ERC20 standard. So chances are there will be not name, symbol etc. fields. Both of these are required as the rewards will be given to the user on the basis of token symbol. And name is used by the other contracts. So these types of assets are not supported.
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
...
_position.EUROs -= costInEuros;
@> rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
...
}
GitHub: 205-241
6. Missing Returns Token
Some tokens doesn't return any value when called. But this is completely different from ERC20 standard. If these types of tokens are used then it would not work if the return value checks are done in the contracts. Try using SafeTransferLib for these type of tokens if used as a collateral.
7. Pausable ERC20s
Some ERC20 has a support for pausing the transfer functionality until some time. If these types of tokens are used as a collateral then it would not be possible to distribute the liquidated assets until it is unpaused. And some staker might unstakee before than causing the loss of rewards. Also these are not supported in Smart vaults.
8. ERC777 tokens
ERC777 tokens has built in hooks that is triggered before the actual transfer of token is done. That mean a user can reenter a function before the actual transfer of funds is done an can cause accounting issue and double spend issues.
