Reentrancy in the manage
function
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: