Bahurum - Loan can be written off by anybody before overdue delay expires

[sherlock-admin]

Bahurum

high

Loan can be written off by anybody before overdue delay expires

Summary

When a borrower takes a second loan after a loan that has been written off, this second loan can be written off instantly by any other member due to missing update of last repay block, leaving the staker at a loss.

Vulnerability Detail

1. A staker stakes and vouches a borrower
2. the borrower borrows calling UToken:borrow: accountBorrows[borrower].lastRepay is updated with the current block number
3. the staker writes off the entire debt of the borrower calling UserManager:debtWriteOff. In the internal call to UToken:debtWriteOff the principal is set to zero but accountBorrows[borrower].lastRepay is not updated
4. 90 days pass and a staker vouches for the same borrower
5. the borrower borrows calling UToken:borrow: accountBorrows[borrower].lastRepay is not set to the current block since non zero and stays to the previous value.
6. accountBorrows[borrower].lastRepay is now old enough to allow the check in
   UserManager:debtWriteOff at line 738 to pass. The debt is written off by any other member immediatly after the loan is given. The staker looses the staked amount immediatly.

        if (block.number <= lastRepay + overdueBlocks + maxOverdueBlocks) {
            if (staker != msg.sender) revert AuthFailed();
        }

7. The last repay block is still stale and a new loan can be taken and written off immediatly many times as long as stakers are trusting the borrower

Note that this can be exploited maliciously by the borrower, who can continously ask for loans and then write them off immediatly.

Impact

The staker of the loan looses the staked amount well before the overdue delay is expired

Code Snippet

https://github.com/sherlock-audit/2022-10-union-finance/blob/main/union-v2-contracts/contracts/market/UToken.sol#L666-L672

    function debtWriteOff(address borrower, uint256 amount) external override whenNotPaused onlyUserManager {
        uint256 oldPrincipal = getBorrowed(borrower);
        uint256 repayAmount = amount > oldPrincipal ? oldPrincipal : amount;

        accountBorrows[borrower].principal = oldPrincipal - repayAmount;
        totalBorrows -= repayAmount;
    }

Tool used

Manual Review

Recommendation

Reset lastRepay for the borrower to 0 when the debt is written off completely

    function debtWriteOff(address borrower, uint256 amount) external override whenNotPaused onlyUserManager {
        uint256 oldPrincipal = getBorrowed(borrower);
        uint256 repayAmount = amount > oldPrincipal ? oldPrincipal : amount;

+       if (oldPrincipal == repayAmount) accountBorrows[borrower].lastRepay = 0;
        accountBorrows[borrower].principal = oldPrincipal - repayAmount;
        totalBorrows -= repayAmount;
    }

[GeraldHost]

unioncredit/union-v2-contracts#64