UNI-1017 loan can be written off by anybody

.github/workflows/ci_audit_fix.yml

@@ -0,0 +1,27 @@
+name: sherlock audit fix tests
+
+on:
+    push:
+        branches:
+            - "audit-fix/**"
+
+jobs:
+    SherlockAuditFixTest:
+        name: Sherlock audit fix tests
+        runs-on: ubuntu-latest
+        steps:
+            - uses: actions/checkout@v3
+              with:
+                  submodules: recursive
+            - uses: actions/setup-node@v3
+              with:
+                  node-version: 14
+
+            - name: Install dependencies
+              run: yarn install
+
+            - name: Compile hardhat
+              run: yarn hh:compile
+
+            - name: Run audit fix tests
+              run: yarn hardhat test test/sherlock-audit-fix/*

contracts/market/UToken.sol

@@ -664,11 +664,22 @@ contract UToken is IUToken, Controller, ERC20PermitUpgradeable, ReentrancyGuardU
     }
 
     function debtWriteOff(address borrower, uint256 amount) external override whenNotPaused onlyUserManager {
+        if (amount == 0) revert AmountZero();
+
         uint256 oldPrincipal = getBorrowed(borrower);
         uint256 repayAmount = amount > oldPrincipal ? oldPrincipal : amount;
 
         accountBorrows[borrower].principal = oldPrincipal - repayAmount;
         totalBorrows -= repayAmount;
+
+        if (repayAmount == oldPrincipal) {
+            // If all principal is written off, we can reset the last repaid block to 0.
+            // which indicates that the borrower has no outstanding loans.
+            accountBorrows[borrower].lastRepay = 0;
+        } else {
+            // Still tracking the exisiting loan.
+            accountBorrows[borrower].lastRepay = getBlockNumber();
+        }
     }
 
     /* -------------------------------------------------------------------

deploy/config/index.ts

@@ -10,6 +10,9 @@ import goerliConfig from "./goerli";
 // Fork Configs
 import goerliForkConfig from "./goerli-fork";
 
+// local test configs
+import localConfig from "./local";
+
 export const baseConfig = {
     addresses: {
         aave: {
@@ -49,6 +52,8 @@ export const getConfig = () => {
             return {...baseConfig, ...goerliConfig};
         case "goerli-fork":
             return {...baseConfig, ...goerliForkConfig};
+        case "local":
+            return {...baseConfig, ...localConfig};
         default:
             return baseConfig;
     }

deploy/config/local.ts

@@ -0,0 +1,22 @@
+import {parseUnits} from "ethers/lib/utils";
+
+import {DeployConfig} from "../index";
+
+export default {
+    userManager: {
+        maxOverdue: "120", // 12 x overdueBlocks
+        effectiveCount: "1",
+        maxVouchers: "1000"
+    },
+    uToken: {
+        name: "uDAI",
+        symbol: "uDAI",
+        initialExchangeRateMantissa: parseUnits("1"),
+        reserveFactorMantissa: parseUnits("1"),
+        originationFee: parseUnits("0.005"),
+        debtCeiling: parseUnits("250000"),
+        maxBorrow: parseUnits("25000"),
+        minBorrow: parseUnits("100"),
+        overdueBlocks: "10" // in blocks
+    }
+} as DeployConfig;

test/sherlock-audit-fix/uni-1017-115-loan-can-be-written-off-by-anybody.ts

@@ -0,0 +1,140 @@
+import {expect} from "chai";
+import {ethers} from "hardhat";
+import {Signer} from "ethers";
+import {commify, formatUnits, parseUnits} from "ethers/lib/utils";
+
+import error from "../utils/error";
+import deploy, {Contracts} from "../../deploy";
+import {getConfig} from "../../deploy/config";
+import {getDai, roll} from "../../test/utils";
+
+const PAD = 22;
+
+describe("Debt write off", () => {
+    let staker: Signer;
+    let stakerAddress: string;
+
+    let borrower: Signer;
+    let borrowerAddress: string;
+
+    let deployer: Signer;
+    let deployerAddress: string;
+
+    let contracts: Contracts;
+
+    const borrowAmount = parseUnits("1000");
+
+    before(async function () {
+        const signers = await ethers.getSigners();
+        deployer = signers[0];
+        deployerAddress = await deployer.getAddress();
+        contracts = await deploy({...getConfig(), admin: deployerAddress}, deployer);
+
+        staker = signers[1];
+        stakerAddress = await staker.getAddress();
+
+        borrower = signers[2];
+        borrowerAddress = await borrower.getAddress();
+    });
+
+    const logInfo = async () => {
+        const principal = await contracts.uToken.getBorrowed(borrowerAddress);
+        const interest = await contracts.uToken.calculatingInterest(borrowerAddress);
+        const owed = await contracts.uToken.borrowBalanceView(borrowerAddress);
+        const totalBorrows = await contracts.uToken.totalBorrows();
+        const totalRedeemable = await contracts.uToken.totalRedeemable();
+        const totalReserves = await contracts.uToken.totalReserves();
+
+        console.log("");
+        console.log("Principal:".padEnd(PAD), commify(formatUnits(principal)));
+        console.log("Interest:".padEnd(PAD), commify(formatUnits(interest)));
+        console.log("Total Owed:".padEnd(PAD), commify(formatUnits(owed)));
+        console.log("Total Borrows:".padEnd(PAD), commify(formatUnits(totalBorrows)));
+        console.log("Total Redeemable:".padEnd(PAD), commify(formatUnits(totalRedeemable)));
+        console.log("Total reserves:".padEnd(PAD), commify(formatUnits(totalReserves)));
+        console.log("");
+
+        const balance = await contracts.userManager.getStakerBalance(stakerAddress);
+
+        console.log("Staker balance:".padEnd(PAD), commify(formatUnits(balance)));
+        console.log("");
+
+        const lastRepay = await contracts.uToken.getLastRepay(borrowerAddress);
+        console.log("Borrower's last repay:".padEnd(PAD), lastRepay.toString());
+        console.log("");
+    };
+
+    it("set up credit line", async () => {
+        const AMOUNT = parseUnits("10000");
+
+        await contracts.userManager.addMember(stakerAddress);
+        await contracts.userManager.addMember(borrowerAddress);
+
+        await getDai(contracts.dai, deployer, AMOUNT);
+        await getDai(contracts.dai, staker, AMOUNT);
+
+        await contracts.dai.connect(staker).approve(contracts.userManager.address, AMOUNT);
+        await contracts.userManager.connect(staker).stake(AMOUNT);
+
+        await contracts.dai.approve(contracts.uToken.address, AMOUNT);
+        await contracts.uToken.mint(AMOUNT);
+
+        await contracts.userManager.connect(staker).updateTrust(borrowerAddress, AMOUNT);
+
+        const creditLine = await contracts.userManager.getCreditLimit(borrowerAddress);
+        console.log("Credit line:".padEnd(PAD), commify(formatUnits(creditLine)));
+
+        expect(creditLine).gt(0);
+
+        await logInfo();
+    });
+
+    it("borrow", async () => {
+        await contracts.uToken.connect(borrower).borrow(borrowerAddress, borrowAmount);
+        const principal = await contracts.uToken.getBorrowed(borrowerAddress);
+        expect(principal).gt(borrowAmount);
+
+        await logInfo();
+    });
+
+    it("staker writes off debt", async () => {
+        const stakeBalBefore = await contracts.userManager.getStakerBalance(stakerAddress);
+        const principal = await contracts.uToken.getBorrowed(borrowerAddress);
+        await contracts.userManager.connect(staker).debtWriteOff(stakerAddress, borrowerAddress, principal);
+        const stakeBalAfter = await contracts.userManager.getStakerBalance(stakerAddress);
+        expect(stakeBalBefore.sub(stakeBalAfter)).eq(principal);
+
+        await logInfo();
+    });
+
+    it("wait until max overdue time passes", async () => {
+        console.log("\nRoll 200 blocks into the future and borrow again\n");
+        await roll(200); // wait until the max overdue blocks passes
+    });
+
+    it("borrow again", async () => {
+        await contracts.uToken.connect(borrower).borrow(borrowerAddress, borrowAmount);
+        const principal = await contracts.uToken.getBorrowed(borrowerAddress);
+        expect(principal).gt(borrowAmount);
+
+        await logInfo();
+    });
+
+    it("Cannot write off debt by others", async () => {
+        const principal = await contracts.uToken.getBorrowed(borrowerAddress);
+        const resp = contracts.userManager.debtWriteOff(stakerAddress, borrowerAddress, principal);
+        await expect(resp).to.be.rejectedWith(error.AuthFailed);
+
+        await logInfo();
+    });
+
+    it("Can write off debt by the staker", async () => {
+        const stakeBalBefore = await contracts.userManager.getStakerBalance(stakerAddress);
+        const principal = await contracts.uToken.getBorrowed(borrowerAddress);
+        await contracts.userManager.connect(staker).debtWriteOff(stakerAddress, borrowerAddress, principal);
+        const stakeBalAfter = await contracts.userManager.getStakerBalance(stakerAddress);
+        expect(stakeBalBefore.sub(stakeBalAfter)).eq(principal);
+
+        await logInfo();
+    });
+});