Skip to content

Prevent Protocol Fee Collection from Draining Depositor Collateral in LendingContract #457

Description

@thisisouvik

Description

In contracts/lending/src/lib.rs, LendingContract maintains a single pooled SEP-41 token balance per asset, but two independent ledgers are fulfilled from that single balance:

  1. Depositor Collateral: Recorded per user in DataKey::UserCollateral(borrower) and funded via deposit_collateral.
  2. Protocol Fees: Recorded in DataKey::UncollectedFees and credited at loan-creation time (create_loan_request, lines 1381–1388) from scheduled interest, before any loan is approved, funded, or repaid.

When collect_fees (lines 753–802) executes, it checks only whether the contract's total pooled token balance covers UncollectedFees:

let token_client = token::Client::new(&env, &fee_token);
let contract_address = env.current_contract_address();
if token_client.balance(&contract_address) < uncollected {
    panic!("Insufficient fee token balance to collect fees");
}

env.storage()
    .instance()
    .set(&DataKey::UncollectedFees, &0i128);
token_client.transfer(&contract_address, &treasury, &uncollected);

Because native XLM is whitelisted as collateral (scripts/deploy-testnet.ts:606-620) and set as the fee token (scripts/deploy-testnet.ts:697-698), the token balance checked by collect_fees consists of depositors' collateral.

collect_fees succeeds by transferring depositors' collateral to the treasury, while leaving each depositor's balance in DataKey::UserCollateral(borrower) unchanged. When depositors subsequently call withdraw_collateral, the call panics inside the token contract with "balance is not sufficient to spend".

Affected Code & Documentation

  • contracts/lending/src/lib.rs:
    • collect_fees (lines 753–802): Balance guard does not account for collateral liabilities.
    • create_loan_request (lines 1327–1338, 1381–1388): Credits UncollectedFees upfront from unearned scheduled interest without any token inflow.
    • deposit_collateral (lines 1029–1048): The only token inflow into the contract.
    • record_payment (lines 1498–1574): No token transfer on repayment.
  • contracts/treasury/src/lib.rs: collect_protocol_fees (lines 178–195).
  • scripts/deploy-testnet.ts: lines 606–620 (whitelists XLM) & lines 697–698 (sets XLM as fee token).
  • docs/apr-formulas.md: Line 373 states that collect_fees "fails if the lending contract does not hold the fee tokens backing the accrual", which is currently violated.

Impact

  1. Direct Loss of Depositor Collateral: Protocol fee collection drains borrower collateral into the treasury, leaving depositor claims unbacked.
  2. Denial of Withdrawal: Depositors cannot exit their positions via withdraw_collateral because the vault holds fewer tokens than the sum of recorded positions.
  3. Fee Ledger Inflation Attack: Any borrower can submit multiple loan requests to artificially inflate UncollectedFees without depositing additional tokens or paying interest, causing the next fee collection to drain up to the entire collateral balance for that asset.

Recommended Mitigation

  1. Track Aggregate Collateral: Maintain a TotalCollateral(asset) accumulator in storage, incremented by deposit_collateral and decremented by withdraw_collateral.
  2. Segregated Balance Guard: In collect_fees, ensure available fee tokens exclude active collateral liabilities:
    let collateral_total = Self::get_total_collateral_for_asset(&env, &fee_token);
    let available_fees = token_client.balance(&contract_address).saturating_sub(collateral_total);
    if available_fees < uncollected {
        panic!("Insufficient unencumbered fee token balance to collect fees");
    }
  3. Accrue Fees on Repayment: Defer UncollectedFees crediting to record_payment when payment tokens are actually received, rather than booking projected interest at loan request creation.
  4. Separate Fee Asset / Account: Disallow setting FeeToken to any whitelisted collateral asset, or hold protocol fees in a segregated custody sub-contract.

Acceptance Criteria

  • Implement TotalCollateral(asset) tracking in contracts/lending/src/lib.rs.
  • Update collect_fees to verify that contract balance strictly exceeds total collateral obligations before executing fee transfers.
  • Accrue protocol fees only when interest payments are settled.
  • Add unit test verifying that collect_fees fails when contract balance consists only of depositor collateral.

Activity

  1. huangji6693-max commented on Oct 26, 2026

    @huangji6693-max

    Hi! I've submitted a fix in #463 that adds DataKey::TotalCollateral(asset) tracking in deposit_collateral / withdraw_collateral, guards collect_fees against encroaching on collateral liabilities (balance.saturating_sub(collateral_total) >= uncollected), defers UncollectedFees accrual to record_payment on repayment, and adds unit test coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions