Lista DAO

Lista DAO: Moolah

Cantina Security Report

Organization

@lista-dao

Engagement Type

Cantina Solo

Period

-

Repositories

GitHublista-dao/moolah

Researchers

slowfi

Findings

Medium Risk

2 findings

2 fixed

0 acknowledged

Informational

4 findings

1 fixed

3 acknowledged

Gas Optimizations

1 findings

0 fixed

1 acknowledged

Medium Risk2 findings

  1. Stale exact asset repay can grief position migrations

    State

    Fixed

    https://github.com/lista-dao/moolah/pull/152/changes/5032d8cbf76f73de806611e257c234d2705d2971

    Severity

    Severity: Medium

    Submitted by

    slowfi

    Description

    The function onMoolahFlashLoan from contract PositionManager repays the source market with MOOLAH.repay(p.outMarket, assets, 0, p.user, ""), where assets is the flash loaned borrowAmount supplied to migrateCommonMarketToFixedTermMarket. This uses Moolah's exact asset repay mode, while IMoolah.repay explicitly warns that a small front run repay can make that flow revert for underflow.

    As a result, a third party can repay a small amount of the user's source debt before execution and cause a full migration that targets the previous debt amount to revert. This permissionlessly blocks the migration until the user resubmits with an updated amount.

    Recommendation

    Consider to repay the source position in shares mode when clearing the migrated debt, or otherwise derive the repay amount from the current position state at execution time instead of relying on a stale caller supplied exact asset amount.

  2. Unconditional native repayment can break integration

    State

    Fixed

    https://github.com/lista-dao/moolah/pull/152/changes/5032d8cbf76f73de806611e257c234d2705d2971

    Severity

    Severity: Medium

    Submitted by

    slowfi

    Description

    The function _transferLoanToken from contract LendingBroker unwraps WBNB and sends native BNB whenever a WBNB provider is registered for the market. Both user facing borrow(uint256) and borrow(uint256,uint256) route through this helper, so contract users that do not implement a payable receive or fallback function revert on borrow instead of receiving the borrowed asset. The same native send path is also used when refunding excess value in both repay functions, which means an otherwise valid repay can revert when any surplus must be returned to a non payable contract caller.

    This changes the standard WBNB broker flow from ERC20 transfers to mandatory native transfers and blocks affected contract integrations.

    Recommendation

    Consider to preserve WBNB ERC20 transfers on the standard borrow and refund paths, and reserve native BNB delivery for an explicit opt-in flow. If native refunds remain supported, consider to avoid unconditional native sends to arbitrary callers and fall back to returning WBNB when the recipient is not expected to receive native BNB.

Informational4 findings

  1. Delegated borrow follows the existing authorization model

    State

    Acknowledged

    Severity

    Severity: Informational

    Submitted by

    slowfi

    Description

    The function borrow from contract LendingBroker allows an authorized caller to open a fixed-term borrow position for user and send the borrowed tokens to an arbitrary receiver, gated by MOOLAH.isAuthorized(user, msg.sender). This does not introduce a new privilege boundary by itself, because IMoolah.isAuthorized is already defined as an approval to modify the author's positions on all markets.

    The behavior is therefore consistent with the existing trust model, but it is worth documenting because integrations may otherwise interpret the new delegated borrow entrypoint as a narrower approval than the underlying Moolah authorization actually provides.

    Recommendation

    Consider to document that delegated fixed term borrowing inherits Moolah's existing all markets authorization scope, and consider to make this trust assumption explicit in user facing documentation and integration guides.

  2. Allowed partial migration

    State

    Acknowledged

    Severity

    Severity: Informational

    Submitted by

    slowfi

    Description

    The function migrateCommonMarketToFixedTermMarket from contract PositionManager accepts caller-controlled collateralAmount and borrowAmount, and the interface and tests already exercise migrations that move only part of the source position. This means the flow is not limited to full migrations: it can intentionally leave residual debt and collateral in the source market as long as the remaining position stays valid after the migration.

    This is an informational behavior note rather than a vulnerability, but it is important to document so users and integrators do not assume the function always closes the source position completely.

    Recommendation

    Consider to document that the migration flow supports both full and partial position moves, and consider to describe the expected post migration health requirements for any residual source position.

  3. Unsolicited native BNB lacks recovery path

    State

    Fixed

    https://github.com/lista-dao/moolah/pull/152/changes/e85ace4148c19a4d54a578ce1d1bcc2b6fe1ecb3

    Severity

    Severity: Informational

    Submitted by

    slowfi

    Description

    The function receive from contract LendingBroker accepts arbitrary native BNB. This is needed for the WBNB unwrap flow, but it also allows accidental transfers and forced native sends to leave a native balance on the broker. The current implementation does not provide a native rescue or sweep function, so unsolicited native BNB cannot be recovered within the current contract version and may remain stranded until an upgrade is performed.

    The impact is limited to operational dust or mistakenly sent funds.

    Recommendation

    Consider to add a manager only native recovery function for unsolicited balances, and consider to document that direct native transfers to the broker are unsupported outside the intended WBNB unwrap flow.

  4. Migration could support a deadline or APR bounds

    State

    Acknowledged

    Severity

    Severity: Informational

    Submitted by

    slowfi

    Description

    The function migrateCommonMarketToFixedTermMarket from contract PositionManager selects the destination product only by termId, while the corresponding fixed-term APR can be updated by the broker bot before execution. As a result, a delayed migration transaction can still execute against the same termId after its APR has changed, without any caller specified deadline or maximum acceptable APR.

    This is a UX hardening note rather than a security issue, but it can lead to a less favorable fixed term than the user expected when the transaction was submitted.

    Recommendation

    Consider to add a deadline and/or maximum acceptable APR to the migration parameters, and consider to validate the current term data against those bounds before starting the flash loan.

Gas Optimizations1 finding

  1. Revert strings can be replaced with custom errors

    State

    Acknowledged

    Severity

    Severity: Gas optimization

    Submitted by

    slowfi

    Description

    The function migrateCommonMarketToFixedTermMarket from contract PositionManager and multiple functions from contract LendingBroker still rely on string-based require and revert messages for input validation and state checks. This pattern is used repeatedly across both contracts for common conditions such as zero address checks, authorization checks, invalid market state, and unsupported operations.

    While this is not a functional issue, replacing repeated revert strings with custom errors would reduce deployed bytecode size and lower revert-path gas costs, while also making shared failure cases easier to standardize across the codebase.

    Recommendation

    Consider to replace repeated revert strings with custom errors, especially for shared validation paths that appear multiple times across PositionManager and LendingBroker. Consider to group common errors in a shared error definition file if the same conditions are used across contracts.