sherlock-audit / 2024-05-elfi-protocol-judging

11 stars 7 forks source link

newt - Reentrancy Vulnerability #285

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

newt

High

Reentrancy Vulnerability

Summary

The transfer function in TranferUtils.sol contains a potential reentrancy vulnerability. Despite using a gas limit, the function makes an external call to transfer ERC-20 tokens without reentrancy protection. This could allow malicious contracts to exploit the reentrancy vulnerability.

Vulnerability Detail

If the recipient of the tokens is a contract with a fallback or receive function, this external call could trigger reentrancy, allowing the contract to call back into the transfer function before the initial execution completes.

Impact

If exploited, a reentrancy attack could allow an attacker to repeatedly call the transfer function before the previous execution completes.

Code Snippet

function transfer(address token, address receiver, uint256 amount) external {
    if (amount == 0) {
        return;
    }
    bool success = transferWithGasLimit(IERC20(token), receiver, amount, TRANSFER_GAS_LIMIT);
    if (!success) {
        revert TokenTransferError(token, receiver, amount);
    }
}

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/main/elfi-perp-contracts/contracts/utils/TransferUtils.sol#L11-L19

Tool used

Manual Review

Recommendation

Add reentrancy guard.

sherlock-admin2 commented 4 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/0xCedar/elfi-perp-contracts/pull/64

nevillehuang commented 4 months ago

Invalid, lack required poc for reentrancy per sherlock rules

sherlock-admin2 commented 3 months ago

The Lead Senior Watson signed off on the fix.