sherlock-audit / 2023-02-bond-judging

2 stars 0 forks source link

Diana - call() should be used instead of transfer() on an address payable #47

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Diana

medium

call() should be used instead of transfer() on an address payable

Summary

Use call() instead of transfer() on an address payable since the use of the deprecated transfer() function for such an address will inevitably make the transaction fail

Vulnerability Detail

The transfer()  function forwards a fixed amount of 2300 gas. Historically, it has often been recommended to use this functions for value transfers to guard against reentrancy attacks. 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:

Code Snippet

https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondBatchAuctionV1.sol#L287

payable(owner()).transfer(address(this).balance);

Tool used

Manual Review

Recommendation

Use call() instead of transfer(), but be sure to respect the CEI pattern and/or add re-entrancy guards, as several hacks already happened in the past due to this recommendation not being fully understood.

More info on;
https://swcregistry.io/docs/SWC-134

Reference reports: https://github.com/sherlock-audit/2022-11-dodo-judging/issues/5

Duplicate of #37

Oighty commented 1 year ago

Duplicate of #37 .