code-423n4 / 2022-12-tigris-findings

8 stars 4 forks source link

Use of `transfer()` instead of `call()` to send eth #585

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/0cb05a462e78c4470662e9d9a4f9ab587f266bb5/contracts/Trading.sol#L588

Vulnerability details

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

Impact

Use of transfer() might render ETH impossible to withdraw because 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.

References

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/2022-12-tigris/blob/0cb05a462e78c4470662e9d9a4f9ab587f266bb5/contracts/Trading.sol#L588 payable(_proxy).transfer(msg.value);

Recommended Mitigation Steps

Use call() to send eth

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #175

c4-judge commented 1 year ago

GalloDaSballo marked the issue as not a duplicate

GalloDaSballo commented 1 year ago

L

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2022-12-tigris-findings/issues/662