code423n4 commented 3 years ago

Handle

JMukesh

Vulnerability details

Impact

Use of transfer might render ETH impossible to withdraw becuase after istanbul hardfork , there is increases in the gas cost of the SLOAD operation and therefore breaks some existing smart contracts.Those contracts will break because their fallback functions used to consume less than 2300 gas, and they’ll now consume more, since 2300 the amount of gas a contract’s fallback function receives if it’s called via Solidity’s transfer() or send() methods. Any smart contract that uses transfer() or send() is taking a hard dependency on gas costs by forwarding a fixed amount of gas: 2300.

https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

https://blog.openzeppelin.com/opyn-gamma-protocol-audit/

Proof of Concept

https://github.com/code-423n4/2021-08-notional/blob/4b51b0de2b448e4d36809781c097c7bc373312e9/contracts/internal/balances/TokenHandler.sol#L174

Tools Used

manual review

Recommended Mitigation Steps

use call() instead of transfer() to send eth

jeffywu commented 3 years ago

This should be 0 (Non critical). ETH transfers still work as of today. This is a best practice recommendation. In the case that there is a hard fork that breaks ETH transfers the protocol can be upgraded to support the use of call()

ghoul-sol commented 3 years ago

I aline with warden on this one. .transfer will not work if the account is a smart contract. Keeping as is.

code-423n4 / 2021-08-notional-findings