# Silo Finance V2 Audit Report – `RepurchaseHook.sol`

**Auditor:** [@zarkk01](https://x.com/zarkk01)
**Date:** 24/3/2025
**Commit Hash:** [81bd334](https://github.com/yielddev/FixedFinance/commit/81bd334733a4a8bb0616d953b54ca6ce98dbadfc)
**Repository:** [FixedFinance](https://github.com/yielddev/FixedFinance)

## Overview

### Hook Overview

The audited smart contract `RepurchaseHook.sol` implements a **fixed-rate, fixed-term loan system** for [Silo Finance V2](https://v2.silo.finance/). Borrowers can take loans in USDC by collateralizing GUSDC PT tokens, which are fixed-yield assets. The design ensures the borrower's exposure is independent of price volatility, as the loan repayment is based on fixed terms and not LTV ratios.

The hook system tracks loan details, deducts upfront interest (`haircut`), and allows for collateral redemption upon full or partial repayment. Liquidation is time-based, not price-based which means that if the term expires without repayment, a liquidator can (permissionless) repurchase the collateral from the borrower.

### Audit Scope

The audit focused on verifying:

- Compatibility with existing **Silo design**.
- Security and correctness of **core logic**.
- Compliance with **gauge incentive system (`GaugeHookReceiver`)**.
- Accurate handling of **loan term expiries and liquidations**.
- Support for **partial repayments and pro-rata interest**.
- Implementation of **upfront fixed interest mechanism and `haircut` deduction**.

### Disclaimer

This audit is not a guarantee of the absence of vulnerabilities. While every reasonable effort has been made to identify and analyze potential issues, smart contracts carry inherent risk. This is a time, resource and expertise bound effort aimed at identifying as many vulnerabilities as possible within the given constraints.

## Findings

| Severity | Count |
|------------|-------|
| High | 3 |
| Medium | 2 |
| Low | 6 |

### High Severity

#### High-1. Missing access control on `beforeAction()` and `afterAction()` leads to permissionless and arbitrary loan creations.
**Location:** [beforeAction()](https://github.com/yielddev/FixedFinance/blob/81bd334733a4a8bb0616d953b54ca6ce98dbadfc/contracts/RepurchaseHook.sol#L65), [afterAction()](https://github.com/yielddev/FixedFinance/blob/81bd334733a4a8bb0616d953b54ca6ce98dbadfc/contracts/RepurchaseHook.sol#L81)
**Impact:** Any smart contract can call `beforeAction()` and create loans for a borrower, effectively stealing his collateral tokens.

**Details:**
`RepurchaseHook::beforeAction()` and `RepurchaseHook::afterAction()` are hook functions triggered when someone interacts with the `underlyingAssetSilo` by borrowing, repaying, or transferring debt shares. However, both functions lack access control and can be called by anyone. This means any contract or EOA can invoke them and, by passing the correct parameters, create a loan for an arbitrary borrower. The loan must then be repaid; otherwise, the victim will be liquidated and their collateral (`PT-gUSDC`) will be seized. This could result in a complete loss of funds for users and the Silos using this `HookReceiver`.

**Recommendation:**
Consider adding access control to the hook functions to ensure only the expected Silo calls them:

    function beforeAction(address _silo, uint256 _action, bytes calldata _inputAndOutput) external {
+       require(msg.sender == underlyingAssetSilo);
        if (Hook.matchAction(_action, Hook.BORROW)) {
            Hook.BeforeBorrowInput memory borrow = Hook.beforeBorrowDecode(_inputAndOutput);
            uint256 haircut = ((loans[borrow.borrower].price + borrow.assets) * 500) / 10_000;
            // ...
    }

    function afterAction(address _silo, uint256 _action, bytes calldata _inputAndOutput) external {
+       require(msg.sender == underlyingAssetSilo);
        if(Hook.matchAction(_action, Hook.BORROW)) {
            // ...
        } else if (Hook.matchAction(_action, Hook.REPAY)) {
            // ...
        } else if (Hook.matchAction(_action, Hook.shareTokenTransfer(Hook.DEBT_TOKEN))) {
            // ...
            }
        }
    }

#### High-2. Loan terms can be bypassed by transferring debt tokens after expiry, allowing borrowers to avoid liquidation indefinitely.
**Location:** [afterAction()](https://github.com/yielddev/FixedFinance/blob/81bd334733a4a8bb0616d953b54ca6ce98dbadfc/contracts/RepurchaseHook.sol#L81)
**Impact:** Borrowers can indefinitely avoid liquidation by transferring debt to another controlled address just before liquidation occurs.

**Details:**
In the `afterAction()` hook, when a debt token transfer (`shareTokenTransfer(Hook.DEBT_TOKEN)`) occurs, the debt and collateral values are migrated to the recipient. However, there is no check to ensure that the loan has not already expired (`term < block.timestamp`). This allows a borrower with an expired loan to front-run their own `liquidationCall()` by simply transferring the debt to another wallet. Since the `liquidator` will be targeting the original `borrower` address, which now holds no expired debt, the liquidation will do nothing. A borrower can repeat this process indefinitely, effectively avoiding liquidation entirely.

**Recommendation:**
Add a check in the share token transfer hook to revert if the sender’s loan term has already expired. This will ensure expired loans cannot be migrated to extend their lifetime.

    if (input.sender != address(0) && input.recipient != address(0)) {
+       require(loans[input.sender].term >= block.timestamp, "Loan term expired");

        // ...
    }

#### High-3. Liquidation reverts indefinitely if collateral is deposited in `PROTECTED_TYPE` Silo.
**Location:** [liquidateCall()](https://github.com/yielddev/FixedFinance/blob/81bd334733a4a8bb0616d953b54ca6ce98dbadfc/contracts/RepurchaseHook.sol#L157)
**Impact:** If a borrower deposits collateral using `PROTECTED_TYPE` instead of regular `COLLATERAL_TYPE`, the liquidation will always revert, making the loan effectively un-liquidatable.

**Details:**
In the `liquidationCall()` function, the hook attempts to transfer the borrower’s collateral to the liquidator via `_callShareTokenForwardTransferNoChecks()`, using the `collateralConfig.collateralShareToken` as the share token for the transfer. However, if the borrower deposited their collateral into the `PROTECTED_SHARES` type instead, this call will revert, since the function cannot transfer protected shares. There is no fallback or alternative logic to handle this case.

        // sieze collateral 
        _callShareTokenForwardTransferNoChecks(
            collateralConfig.silo,
            _borrower,
            shareTokenReceiver,
            loan.collateral,
@>            collateralConfig.collateralShareToken,
@>            ISilo.AssetType.Collateral
        );

**Recommendation:**
Consider adding the flexibility for the liquidator to select which type of collateral they will liquidate. Also, make sure that `transitionCollateral()` from one type to the other is prohibited if the `loan.term` is expired.

---

### Medium Severity

#### Medium-1. Borrowers pay `haircut` repeatedly for previously borrowed debt.
**Location:** [afterAction()](https://github.com/yielddev/FixedFinance/blob/81bd334733a4a8bb0616d953b54ca6ce98dbadfc/contracts/RepurchaseHook.sol#L65)
**Impact:** Borrowers are charged the full haircut again on every new borrow, including on the previously borrowed amount that already had the haircut deducted. This results in users overpaying fees and receiving significantly less USDC than expected.

**Details:**
In the `beforeAction()` hook, the haircut is calculated as a percentage of the entire cumulative loan (`loans[borrow.borrower].price + borrow.assets`). This means that for every new borrow, the borrower pays a haircut on both the new amount and the previous debt that already had a haircut applied. This leads to compounding haircut fees on the same loan amount and penalizes users who borrow in multiple transactions.

    uint256 haircut = (loans[borrow.borrower].price + borrow.assets) * 500 / 10_000;

For example, if a user first borrows 100 USDC and then borrows an additional 100 USDC, the second borrow will charge a haircut on 200 USDC, resulting in 15 USDC in total haircut fees (5 + 10), instead of just 10.

**Recommendation:**
Only apply the haircut to the newly borrowed amount in each transaction. Update the calculation as follows:

-   uint256 haircut = (loans[borrow.borrower].price + borrow.assets) * 500 / 10_000;
+   uint256 haircut = borrow.assets * 500 / 10_000;

This ensures borrowers are charged a consistent fixed fee on each new loan amount and are not penalized for borrowing in multiple steps.

#### Medium-2. Potential sandwich opportunity during USDC `haircut` redistribution.
**Location:** [afterAction()](https://github.com/yielddev/FixedFinance/blob/81bd334733a4a8bb0616d953b54ca6ce98dbadfc/contracts/RepurchaseHook.sol#L81)
**Impact:** A malicious LP may be able to front-run and sandwich the `borrow()` call to capture a portion of the redistributed `USDC` from the `haircut`, reducing fairness for lenders.

**Details:**
When a borrower takes a loan, the hook deducts a 5% haircut and immediately redeposits that haircut back into the Silo. This `USDC` is then distributed to LPs by minting and burning share tokens in a single transaction:

// 1. Transfer haircut from borrower
IERC20(ISilo(_silo).asset()).safeTransferFrom(borrow.receiver, address(this), haircut);

// 2. Deposit haircut into Silo
IERC20(ISilo(_silo).asset()).approve(_silo, haircut);
uint256 shares = ISilo(_silo).deposit(haircut, address(this));

// 3. Burn shares to redistribute haircut to LPs
(bool success, bytes memory data) = ISilo(_silo).callOnBehalfOfSilo(
    _silo,
    0,
    ISilo.CallType.Call,
    abi.encodeWithSelector(IShareToken.burn.selector, address(this), address(this), shares)
);

Since this haircut is redistributed instantly, any LP that enters the pool just before the borrow transaction is executed will receive a share of that haircut without contributing any liquidity themselves. This opens up a opportunity where a bot can front-run the borrow, deposit a lot of liquidity, receive a portion of the redistributed haircut, and exit afterward, essentially farming `haircut` rewards unfairly.

**Recommendation:**
Fix for this may be non trivial.

---

### Low Severity

#### Low-1. Unused `owner` variable in `initialize()`.
The `initialize()` function decodes an `owner` address from calldata but does not use it anywhere in the contract. If the `owner` is unnecessary for this hook’s logic, consider removing it from both calldata and the function logic.

#### Low-2. No check for zero-term loans during `liquidationCall()`.
In `liquidationCall()`, the contract doesn’t explicitly handle the case where `loan.term == 0`. Since this implies there’s no active loan to liquidate, it would be cleaner to revert or return early in that case to avoid ambiguity.

#### Low-3. `closeOutLoan()` call is redundant after full repayment.
In `liquidationCall()`, `closeOutLoan()` is called at the end, but the borrower’s debt has already been repaid via `ISilo().repay()`. Since the repayment hook already checks and closes the loan when `price == 0` in `afterAction()`, this second call is redundant and can be removed for clarity.

#### Low-4. Unused custom errors.
The contract defines multiple custom errors such as `NoDebtToCover`, `UnknownRatio()`, and `NoRepayAssets()` which are never used throughout the contract logic. It’s recommended to remove them if they’re not planned for future use.

#### Low-5. Outdated comment about extra debt minting.
The comment `// mint extra debt to represent the haircut` in `afterAction()` is inaccurate. The haircut amount is transferred from the borrower, deposited back into the silo, and then burned, but no new debt is minted. Updating it to accurately reflect the redistribution process is recommended.

#### Low-6. Loan term can be extended by reborrowing 1 wei.
If the bug related to repeated haircut charges (M1) is fixed (so that haircut is only paid on the new amount), a user could extend their loan’s expiration by borrowing a trivial amount (e.g., 1 wei). This would reset the term and avoid liquidation. While this relies on another issue being fixed, and may be an intentional design decision, for this reason is presented as low severity one. In other way, it may be High one.

## Requirements list

Below are the key audit goals as requested by team and whether they were satisfied:

### Hook code does not collide with current Silo design.
> No observed collisions. Complies with `BaseHookReceiver` and uses hook configuration correctly. Assuming the Silo market that integrates with this hook is properly configured (regarding `InterestRateModelV2`, oracles, etc.), and the tokens in the Silo are `PT-gUSDC` and `USDC` or a similar combo, this hook does not collide with the current Silo design.

### Hook does not introduce security issues.
> Issues noted in the findings above. All are patchable and clearly scoped. In particular, the `High` and `Medium` severity issues are crucial to fix. Also, considering the 2 future designs of the `README`, additional review may be necessary.

### Hook supports gauge incentives.
> This implementation of `RepurchaseHook.sol` is not connected with gauge incentives in any way. It only inherits from `BaseHookReceiver` and nothing more. However, future support for gauge incentives is possible, but that would require several code changes to the hook and an additional review would be necessary.
> ```solidity
> contract RepurchaseHook is BaseHookReceiver {
> ```

### All positions become insolvent after term is up.
> That is correct. The `require` statement at the beginning of `liquidationCall()` ensures that once the term of a loan has expired, it can be liquidated and is considered insolvent.
> ```solidity
> function liquidationCall( // solhint-disable-line function-max-lines, code-complexity
> address _collateralAsset,
> address _debtAsset,
> address _borrower
> )
> external
> virtual
> returns (uint256 withdrawCollateral, uint256 >repayDebtAssets)
> {
>
> Loan memory loan = loans[_borrower];
> require(loan.term < block.timestamp, "Loan is still active");
> ```

### Hook supports partial liquidations before term ends.
> Before the term ends, a borrower can partially repay their loan. If, after the term expires, they haven’t repaid the remaining amount, they will be liquidated only on the remaining part which works as expected. However, before the term ends, no liquidation by a liquidator can take place.

### Fixed interest is taken upfront and can be defined at fixed APR.
> The `haircut`, which is essentially the upfront fixed interest, is indeed taken in the `afterBorrow()` function. It immediately collects this fee from the `receiver` and redeposits it back into the pool — so yes, the fixed interest is taken upfront. Currently, it is fixed at 500 bps (5%), but it is not calculated as an APR or anything dynamic — just a flat 5% of the borrowed amount.
> ```solidity
> Loan memory loan = loans[borrow.borrower];
> uint256 haircut = ((loan.price) * 500) / 10_000;
> if (haircut > 0) { // mint extra debt to represent the haircut. IE repayment is 100usd but 97usd was delivered, 3 usd haircut
> IERC20(ISilo(_silo).asset()).safeTransferFrom(borrow.receiver, address(this), haircut);
> IERC20(ISilo(_silo).asset()).approve(_silo, haircut);
> uint256 shares = ISilo(_silo).deposit(haircut, address(this));
> (bool success, bytes memory data) = ISilo(_silo).callOnBehalfOfSilo(_silo,
> uint256(0), ISilo.CallType.Call, abi.encodeWithSelector(IShareToken.burn.selector, address(this), address(this), shares));
> if (!success) {
> revert("Haircut debt issuance failed");
> // handle unloan
> }
> }
> ```

### Hook supports full liquidation after the term is up.
> The liquidator can **only** fully repay and receive all collateral after the loan term has expired. As seen in the `liquidationCall()` function signature, there is no parameter for specifying an amount and the call will attempt to liquidate the entire `loan.price` amount from the borrower.
> ```solidity
> function liquidationCall( // solhint-disable-line function-max-lines, code-complexity
> address _collateralAsset,
> address _debtAsset,
> address _borrower
> )
> ```

### Interest rate is pro-rated to term.
> Currently, fixed fee is flat (5%) and **not** dynamically pro-rated to term.
>```solidity
>uint256 haircut = ((loan.price) * 500) / 10_000;
>```