Omniscia fetchai Audit

LendingPool Manual Review Findings

LendingPool Manual Review Findings

LPL-01M: Inexistent Access Control

Description:

The linked function performs no access control on the caller, permitting the finalization of a loan transfer at any point in time by anyone.

Example:

contracts/ALP/LendingPool.sol
823/**
824 * @notice Finialise the transferring of a loan - do the actual ACT transfer
825 * this is part 2 of the process
826 * @param fromACTId Id of the old ERC1155 ACT token that act will be taken from the system
827 * @param toACTId Id of the new ERC1155 ACT token that act will be transferred into the system
828 * @param trustee address of trustee to transfer from and to
829 */
830function transferLoanACT(
831    uint256 fromACTId,
832    uint256 toACTId,
833    uint256 actAmount,
834    address trustee
835) external whenNotPaused() {
836    // Register our new ACT with the breach monitor
837    breachMonitorContract.registerACT(toACTId, lendingPoolWalletAddress, breachAddress);
838

839    // If withdrawing the last of the ACT, then unregister ourselves form the breach monitor
840    if (actContract.balanceOf(lendingPoolWalletAddress, fromACTId) == actAmount)
841        breachMonitorContract.unregisterACT(fromACTId);
842

843    actContract.safeTransferFrom(lendingPoolWalletAddress, trustee, fromACTId, actAmount, "");
844    actContract.safeTransferFrom(trustee, lendingPoolWalletAddress, toACTId, actAmount, "");
845}

Recommendation:

We advise strict access control to be imposed on this function to ensure that it cannot be invoked arbitrarily.

Alleviation:

Access control was properly introduced to the transferLoanACT function by invoking the internal requireIsPrivileged function.

LPL-02M: Incorrect WithdrawACT Event Argument

TypeSeverityLocation
Logical FaultMinorLendingPool.sol:L649

Description:

The WithdrawACT event is currently emitting the caller of the withdrawACT function instead of the account that the withdrawal is performed for.

Example:

contracts/ALP/LendingPool.sol
615function withdrawACT(
616    address account,
617    uint256 actId,
618    uint256 withdrawACTAmount
619) external whenNotPaused() nonReentrant() {
620    requireIsPrivileged();
621

622    (uint256 actBalance, ) = lendingPoolStorageContract.getLoanDetails(account, actId);
623    require(actBalance != 0, "LendingPool: No loan found");
624    require(withdrawACTAmount <= actBalance);
625

626    // Check that if we were to do the withdrawal we would be within our limit
627    require(
628        getBorrowerLimitWithACT(actBalance - withdrawACTAmount, actId) >=
629            getBorrowerDebt(account, actId),
630        "LendingPool: Exceeds withdraw limit"
631    );
632

633    lendingPoolStorageModifierContract.decACT(account, actId, withdrawACTAmount);
634

635    // If withdrawing the last of the ACT, then unregister ourselves form the breach monitor
636    if (actContract.balanceOf(lendingPoolWalletAddress, actId) == withdrawACTAmount)
637        breachMonitorContract.unregisterACT(actId);
638

639    // Note that there is a danger here for reentrancy attack via msg.sender implementing
640    // onReceiveEC1155. However, the Checks-Effects-Interactions Pattern should prevent this
641    actContract.safeTransferFrom(
642        lendingPoolWalletAddress,
643        account,
644        actId,
645        withdrawACTAmount,
646        ""
647    );
648

649    emit WithdrawACT(lendingPoolWalletAddress, msg.sender, actId, withdrawACTAmount);
650

651    // note that due to reentrancy we may not exit this funtion within our borrowing limit,
652    // so do not call it assuming this to be the case
653}

Recommendation:

We advise the event argument to be corrected to ensure that off-chain processes perform as expected.

Alleviation:

The WithdrawACT event now emits a proper account argument rather than the msg.sender.

LPL-03M: Potential of Zero Amount Withdrawals

Description:

The withdrawACT function does not validate the requested amount for the withdrawal, permitting re-entrancies to manifest arbitrarily even for zero-value transfers.

Example:

contracts/ALP/LendingPool.sol
615tion withdrawACT(
616address account,
617uint256 actId,
618uint256 withdrawACTAmount
619ternal whenNotPaused() nonReentrant() {
620requireIsPrivileged();
621

622(uint256 actBalance, ) = lendingPoolStorageContract.getLoanDetails(account, actId);
623require(actBalance != 0, "LendingPool: No loan found");
624require(withdrawACTAmount <= actBalance);

Recommendation:

We advise a require check to be introduced ensuring that a non-zero ACT amount is attempted to be withdrawn.

Alleviation:

A new require check was introduced in the codebase that ensures the withdrawACTAmount is non-zero thus alleviating this exhibit.

Prev

InterestManager.sol (IMR-M)

Next

LoanLiquidator.sol (LLR-M)

On this page