Escher contest - karanctf's results

Lines of code

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

Vulnerability details

Vulnerability Detail

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. 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:

• The claimer smart contract does not implement a payable function.
• The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit.
• The claimer 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.

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

Lines of code

https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L101

Vulnerability details

Summary

Unsafe typecast on from uint256 to uint80 in refund() funciton in LPDA.sol

Impact

There is a risk of overflow or underflow if the uint256 value is too large or too small to be represented by a uint80 type. This could result in unpredictable behavior or vulnerabilities in the contract.

  uint80 price = uint80(getPrice()) * r.amount;
    

In above expression it calls getPrice() which returns uint256.

Recommendation

Use SafeCast library in OpenZeppelin which is a utility library that provides safe and reliable methods for casting between different numeric types in Solidity.