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

0 stars 0 forks source link

`transfer` is used on address payable #509

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-escher/blob/0cf28046e8fe79996f912c7cfc40239ebb863255/src/minters/FixedPrice.sol#L109 https://github.com/code-423n4/2022-12-escher/blob/0cf28046e8fe79996f912c7cfc40239ebb863255/src/minters/OpenEdition.sol#L92 https://github.com/code-423n4/2022-12-escher/blob/0cf28046e8fe79996f912c7cfc40239ebb863255/src/minters/LPDA.sol#L86

Vulnerability details

Proof of Concept

The codebase makes heavy use of the deprecated transfer function of address payable. Its will inevitably make the transaction fail when:

  1. The receiver smart contract does not implement a payable function.
  2. The receiver smart contract does implement a payable fallback which uses more than 2300 gas unit.
  3. The receiver smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call's gas usage above 2300.

Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.

This vulnerability can result in a permanent DoS if the receiver address is of the above mentioned types.

Severity

The impact of this issue is high, since token will be stuck forever, and likelihood is Low/Med since EOAs and most smart contracts will not have this problem. This results in Medium severity.

Recommendation

Use call with value instead of transfer on address payable

c4-judge commented 1 year ago

berndartmueller marked the issue as duplicate of #99

c4-judge commented 1 year ago

berndartmueller marked the issue as satisfactory