Skip to content

Conversation

@Hazim-om
Copy link
Contributor

Rounding Logic Flaws in Division Operations

  • Severity: High
  • Target: Smart Contract
  • Category: Precision Issue
  • Status: Pending

Description

The program uses floor-rounding division checked_div across nearly all arithmetic operations. This behavior can be unsafe in certain scenarios, below are two examples.

    let shares = (amount as u128)
        .checked_mul(pair.total_debt0_shares as u128)
        .ok_or(ErrorCode::DebtShareMathOverflow)?
        .checked_div(pair.total_debt0 as u128)
        .ok_or(ErrorCode::DebtShareDivisionOverflow)?
        .try_into()
        .map_err(|_| ErrorCode::DebtShareDivisionOverflow)?;
    pair.total_debt0_shares = pair.total_debt0_shares.saturating_add(shares);
    self.debt0_shares = self.debt0_shares.saturating_add(shares);

During the borrow operation, the user debt shares will be updated. The computed shares may be underestimated, and in edge cases (e.g., amount == 1), the result will always be zero, causing the borrower to receive the borrowed token without taking any debt shares.

    let fee0 = (amount0 as u128)
        .checked_mul(FLASHLOAN_FEE_BPS as u128)
        .unwrap()
        .checked_div(BPS_DENOMINATOR as u128)
        .unwrap() as u64;

Flashloan fees are also computed using floor rounding. This causes the protocol to charge less than the intended fee.

Impact

Users may gain unintended benefits, resulting in a loss to the protocol and LPs.

Recommendation

Review all division operations across the codebase and adjust rounding behavior according to economic intent. The basic principle is to round in a way that favors the protocol and is unfavorable to the user.

Fix

  • Created ceil_div() in math.rs.
  • Used ceil_div() for protocol-favored rounding.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants