Re-entrancy Vulnerabilities in BaseLSTAdapter

Summary

The contract BaseLSTAdapter.sol implements a Liquid Staking Token Adapter making use of OpenZeppelin's ReentrancyGuard modifier to prevent re-entrancy attacks.

Vulnerability Detail

However, the functions prefundedRedeem(), claimWithdrawal(), setStakingLimit(), pauseStaking(), and unpauseStaking() don't make use of any re-entrancy guarding mechanism. These functions include external calls in the form of token transfers (IWETH9(WETH).transfer()) and could potentially be vulnerable to a re-entrancy attack if called by malicious contracts.

Impact

In specific situations, these external calls might be exploited to call the function again before the first call is finished, possibly leading to unexpected behaviour or loss of funds.

Code Snippet

https://github.com/sherlock-audit/2024-05-napier-update/blob/main/napier-v1/src/adapters/BaseLSTAdapter.sol#L168-L191

/// @notice Handles prefunded redemptions
/// @dev Withdraw from the buffer. If the buffer is insufficient, revert with an error
/// @param recipient The address to receive the redeemed WETH
/// @return The amount of redeemed WETH
/// @return The amount of shares burned
function prefundedRedeem(address recipient) external virtual onlyTranche returns (uint256, uint256) {
    // ...
    /// INTERACT ///
    IWETH9(WETH).transfer(recipient, assets);
    // ...
}

The code snippet from the prefundedRedeem() function shows an external call to transfer tokens to a recipient, which could potentially lead to a re-entrancy attack if the recipient is a malicious contract.

Tool used

Manual Review

Recommendation

Implement the nonReentrant modifier in these functions.

sherlock-admin3 commented 4 months ago

1 comment(s) were left on this issue during the judging contest.

z3s commented:

Invalid; PoC is required for reentrancy attacks.

sherlock-admin2 commented 4 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/napierfi/napier-v1/pull/217