Some users are unable to withdraw or harvest rewards if the recipient is a contract.
Summary
Vulnerability Detail
The issue lies within the harvestPositionTo()and harvestPositionsTo()functions, which are used to harvest rewards from a staking position. The problem arises from the use of safeTransfer()in Solidity, which internally usestransfer().
Here is the relevant code snippet:
function safeTransfer(IERC20 token, address to, uint256 value) internal {
_callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value)));
}
According to the Solidity documentation, transfer() only forwards 2300 gas. This limitation means that if the recipient is a contract with a receive() or fallback() function that consumes more than 2300 gas, the transaction will revert.
Additionally, when creating a position, the user receives an LsNFT, which is transferable. If the user transfers to a contract requiring more than 2300 gas, they will be unable to perform any functions. In such a case, the user will need to transfer the position back to an externally owned account (EOA).
ERC20 token transfer is different from Ether transfer. The transfer function usually provides enough gas and the 2300 gas limit does not apply to token transfers.
karsar
Medium
Some users are unable to withdraw or harvest rewards if the recipient is a contract.
Summary
Vulnerability Detail
The issue lies within the
harvestPositionTo()
andharvestPositionsTo()
functions, which are used to harvest rewards from a staking position. The problem arises from the use ofsafeTransfer()
in Solidity, which internally usestransfer()
.Here is the relevant code snippet:
According to the Solidity documentation, transfer() only forwards 2300 gas. This limitation means that if the recipient is a contract with a receive() or fallback() function that consumes more than 2300 gas, the transaction will revert.
Additionally, when creating a position, the user receives an LsNFT, which is transferable. If the user transfers to a contract requiring more than 2300 gas, they will be unable to perform any functions. In such a case, the user will need to transfer the position back to an externally owned account (EOA).
Impact
inconvenience for users
Code Snippet
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L451-#L490
Tool used
Manual Review
Recommendation
use call() instead of transfer().