search

code-423n4 / 2021-05-fairside-findings

0 stars 0 forks source link

Solidity keyword `transfer` is used in the contract `Withdrawable` #67

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

shw

Vulnerability details

Impact

The function withdraw in the contract Withdrawable uses the Solidity keyword, transfer, which is unrecommended since it forwards a fixed amount of 2300 gas to the recipient. The gas cost of opcodes may change during hard forks in the future and thus break the functionalities of existing deployed contracts.

Proof of Concept

Referenced code: Withdrawable.sol#L18

Please refer to the following references for more details:

Solidity issue - Remove .send and .transfer Stop Using Solidity's transfer() Now

Recommended Mitigation Steps

Use .call{value: 1 ether}("") instead of transfer or send. Besides, since the call function forwards all gas to the recipient, the contract should add protections (e.g., reentrancy guards) to prevent the recipient from reentering critical functions.

fairside-core commented 3 years ago

Although I am fine with the severity, perhaps it may not be applicable given that even after EIP-3074 transfers will not fail with proper access lists and I highly doubt the transfer method will fail working altogether anytime soon.

fairside-core commented 3 years ago

Fixed in PR#8.