hats-finance / Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573

0 stars 1 forks source link

Use call instead of transfe. #32

Open hats-bug-reporter[bot] opened 9 months ago

hats-bug-reporter[bot] commented 9 months ago

Github username: @hunter-w3b Twitter username: hunter_w3b Submission hash (on-chain): 0x249cf5faaaf682d96f736ef01511defb25179f1876f87ed932b8b46fef040dce Severity: medium

Description: Description\ In the forwardETH functions, transfer() is used for native ETH . The transfer() and send() functions forward a fixed amount of 2300 gas. Historically, it has often been recommended to use these functions for value transfers to guard against reentrancy attacks but this function is only called by Master. However, the gas cost of EVM instructions may change significantly during hard forks which may break already deployed contract systems that make fixed assumptions about gas costs. For example. EIP 1884 broke several existing smart contracts due to a cost increase of the SLOAD instruction.

impact\ The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:

PendlePowerFarmController.sol::forwardETH

    function forwardETH(
        address _to,
        uint256 _amount
    )
        external
        onlyMaster
    {
        payable(_to).transfer(
            _amount
        );
    }

Recommended Mitigation\ Use call() instead of transfer().

(bool result, ) = payable(_to).call{value: _amount}("");
 require(result, "Failed to send Ether");
vm06007 commented 9 months ago

This does not seems like a bug or a medium to me, 1) forwardETH is not used anywhere internally 2) forwardETH can only be used by master wallet 3) forwardETH is similar to skim function allowing to get any stuck ETH in the PendlePowerFarmController

This seems nearly a suggestion to use payable(_to).call but does not qualify as medium.