Assessment reports>Maia DAO Ulysses Protocol>Low findings>Reentrancy in the ,manage, function
Category: Coding Mistakes

Reentrancy in the manage function

Low Severity
Low Impact
Low Likelihood

Description

The manage function permits trusted strategy contracts to borrow funds from the BranchPort contract. However, a strategy contract is restricted from withdrawing more token funds than its reserve management limit. Therefore, the total of funds already borrowed plus the new amount must not exceed this limit.

function _enforceReservesLimit(
    address _token,
    uint256 _amount,
    uint256 _strategyTokenDebt,
    uint256 _portStrategyTokenDebt
) internal view {
    ...
    if ((_amount + _portStrategyTokenDebt) > _strategyReserveManagementLimit(_token, totalTokenBalance)) {
        revert ExceedsReserveRatioManagementLimit();
    }
}

function _strategyReserveManagementLimit(address _token, uint256 _totalTokenBalance)
    internal
    view
    returns (uint256)
{
    return
        (_totalTokenBalance * (DIVISIONER - strategyReserveRatioManagementLimit[msg.sender][_token])) / DIVISIONER;
}

The replenishReserves function enables the return of funds from the strategy back to the BranchPort contract. Additionally, the getPortStrategyTokenDebt and getStrategyTokenDebt amounts are reduced prior to the external withdraw call. Since the manage function lacks a lock modifier to prevent reentrancy attacks, a strategy contract may reenter the manage function, potentially increasing the amount of funds it can borrow.

function replenishReserves(address _token, uint256 _amount) external override lock {
    getPortStrategyTokenDebt[msg.sender][_token] -= _amount;
    getStrategyTokenDebt[_token] -= _amount;

    uint256 currBalance = ERC20(_token).balanceOf(address(this));

    IPortStrategy(msg.sender).withdraw(address(this), _token, _amount);

    require(ERC20(_token).balanceOf(address(this)) - currBalance == _amount, "Port Strategy Withdraw Failed");
    emit DebtRepaid(msg.sender, _token, _amount);
}

Impact

After an external call is executed, the replenishReserves function verifies the balance, requiring the strategy to return any additional borrowed funds by the end of the transaction. Consequently, the impact is not critical, as only trusted strategy contracts have the capability to invoke the manage function.

Recommendations

Given that the potential for an attack remains a threat to the contract's security, we advise implementing reentrancy attack safeguards within the manage function. Additionally, it is recommended to reduce the amounts of getPortStrategyTokenDebt and getStrategyTokenDebt subsequent to the external call.

Remediation

This issue has been acknowledged by Maia DAO, and fixes were implemented in the following commits:

Zellic © 2024Back to top ↑