Summary
ThunderLoan::setAllowedToken() lags check whether the token has a pool or not. Fees is calculated when the user takes flash loan on the basis of price of token in weth, which is brought from oracle pool but if the token don't have a pool then user can't take flash loan as it will always revert on fee calculation because the pool will be equal to address(0) and it will get reverted as no function calls can be made on address(0). Thus, no user can take flash loan and it is necessary that there is a check in setAllowedToken to revert if there is no pool corresponding to token.
Vulnerability Details
If the token being allowed in the protocol don't have a pool, then fees can never be calculated as it uses the weth price corresponding to that token which is fetched from pool. But if there is no pool for a token then fees can never be calculated and all functions which involves fee calculation will always revert.
NOTE: This vulnerability is present in both ThunderLoan and ThunderLoanUpgraded contract.
PoC
contract BaseTest is Test {
ThunderLoan thunderLoanImplementation;
MockPoolFactory mockPoolFactory;
ERC1967Proxy proxy;
ThunderLoan thunderLoan;
ERC20Mock weth;
ERC20Mock tokenA;
+ ERC20Mock tokenB;
function setUp() public virtual {
thunderLoan = new ThunderLoan();
mockPoolFactory = new MockPoolFactory();
weth = new ERC20Mock();
tokenA = new ERC20Mock();
+ tokenB = new ERC20Mock();
mockPoolFactory.createPool(address(tokenA));
proxy = new ERC1967Proxy(address(thunderLoan), "");
thunderLoan = ThunderLoan(address(proxy));
thunderLoan.initialize(address(mockPoolFactory));
}
}
modifier setAllowedTokenB_AndMintToken() {
thunderLoan.setAllowedToken(tokenB, true);
tokenB.mint(liquidityProvider, DEPOSIT_AMOUNT);
_;
}
function test_deposit_RevertsIf_TokenAllowlistedHasNoPool() public setAllowedTokenB_AndMintToken {
vm.startPrank(liquidityProvider);
tokenB.approve(address(thunderLoan), DEPOSIT_AMOUNT);
vm.expectRevert();
thunderLoan.deposit(tokenB, DEPOSIT_AMOUNT);
vm.stopPrank();
}
Impact
User can't call the function which uses the fees calculation as pool is used to calculate weth amount corresponding to that token and if there is no pool, then function call will always revert.
Tools Used
Manual Review, Foundry Tests
Recommendations
To have a check in setAllowedToken() function, that if there is no pool corresponding to the token then it should revert.
@@ -25,7 +25,7 @@ contract OracleUpgradeable is Initializable {
return getPriceInWeth(token);
}
- function getPoolFactoryAddress() external view returns (address) {
+ function getPoolFactoryAddress() public view returns (address) {
return s_poolFactory;
}
}
@@ -72,6 +72,7 @@ import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/I
import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import { OracleUpgradeable } from "./OracleUpgradeable.sol";
import { Address } from "@openzeppelin/contracts/utils/Address.sol";
+import { IPoolFactory } from "../interfaces/IPoolFactory.sol";
contract ThunderLoan is Initializable, OwnableUpgradeable, UUPSUpgradeable, OracleUpgradeable {
error ThunderLoan__NotAllowedToken(IERC20 token);
@@ -83,6 +84,7 @@ contract ThunderLoan is Initializable, OwnableUpgradeable, UUPSUpgradeable, Orac
error ThunderLoan__ExhangeRateCanOnlyIncrease();
error ThunderLoan__NotCurrentlyFlashLoaning();
error ThunderLoan__BadNewFee();
+ error ThunderLoan__TokenHasNoPool();
using SafeERC20 for IERC20;
using Address for address;
@@ -229,6 +231,9 @@ contract ThunderLoan is Initializable, OwnableUpgradeable, UUPSUpgradeable, Orac
if (address(s_tokenToAssetToken[token]) != address(0)) {
revert ThunderLoan__AlreadyAllowed();
}
+ if (IPoolFactory(getPoolFactoryAddress()).getPool(address(token)) == address(0)) {
+ revert ThunderLoan__TokenHasNoPool();
+ }
string memory name = string.concat("ThunderLoan ", IERC20Metadata(address(token)).name());
string memory symbol = string.concat("tl", IERC20Metadata(address(token)).symbol());
AssetToken assetToken = new AssetToken(address(this), token, name, symbol);
