Description

The depositFor function uses before- and after-balance checks during token transfers to account for the actual amount of transferred tokens.

function depositFor(address _token, address _for, uint256 _amount) whenNotPaused external {
    if (_amount == 0) revert DepositAmountCannotBeZero();
    if (_for == address(0)) revert CannotDepositForZeroAddress();
    if (!tokenAllowlist[_token]) revert TokenNotAllowedForStaking();
    if (capsEnabled && caps[_token] < getTokenTotalStaked(_token) + _amount) revert CapReached();
    
    uint256 balanceBefore = IERC20Metadata(_token).balanceOf(address(this));
    IERC20Metadata(_token).safeTransferFrom(msg.sender, address(this), _amount);
    uint256 actualAmount = IERC20Metadata(_token).balanceOf(address(this)) - balanceBefore;

    emit Deposit(++eventId, _for, _token, actualAmount);

    ReceiptToken(tokenMap[_token]).mint(_for, actualAmount);
}

However, tokens that support callbacks during transfer (such as ERC-777) can reenter the depositFor function, compromising ulterior calculations.

For example, a user transfers 100 tokens during a depositFor call. The initial value of balanceBefore is 0. If a callback function is invoked in the token contract and the user reenters the depositFor() function, the new balanceBefore would be equal to 100 tokens. During the reentered call, another 100 tokens would be transferred, resulting in the actualAmount being calculated as 200 minus balanceBefore (which is 100), giving an actualAmount of 100.

Meanwhile, in the initial call, the actualAmount would be calculated as 200 minus balanceBefore (which is 0), resulting in an actualAmount of 200. This creates a situation where the total accounted token amount is 300, even though only 200 tokens have actually been transferred.

Impact

This issue can lead to an inflated actualAmount value, which impairs the correct accounting of the total staked tokens.

Recommendations

We recommend implementing a reentrancy-protection mechanism to prevent such attacks, especially if tokens with callback support are allowed to be used with the SatlayerPoolV2 contract.

Remediation

This issue has been acknowledged by SatLayer, and a fix was implemented in commit 07f66277↗. In addition, the SatLayer team has stated that they do not plan to integrate such tokens, and they will be mindful of this issue in the future.