code-423n4 / 2024-01-curves-findings

1 stars 0 forks source link

Avoid using transfer() to send ether #1444

Closed c4-bot-1 closed 9 months ago

c4-bot-1 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/main/contracts/FeeSplitter.sol#L85 https://github.com/code-423n4/2024-01-curves/blob/main/contracts/FeeSplitter.sol#L116

Vulnerability details

Impact

Avoid transfer() as reentrancy mitigations: Although transfer() have been recommended as a security best-practice to prevent reentrancy attacks because they only forward 2300 gas, the gas repricing of opcodes may break deployed contracts.

Proof of Concept

https://github.com/code-423n4/2024-01-curves/blob/main/contracts/FeeSplitter.sol#L85 https://github.com/code-423n4/2024-01-curves/blob/main/contracts/FeeSplitter.sol#L116 Natural gas costs may change, then the smart contract cannot rely on any specific gas cost.

https://eips.ethereum.org/EIPS/eip-1884 https://consensys.io/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Tools Used

Manual review

Recommended Mitigation Steps

Use call() instead, without hardcoded gas limits along with checks-effects-interactions pattern or reentrancy guards for reentrancy protection.

Assessed type

Token-Transfer

c4-pre-sort commented 9 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 9 months ago

raymondfam marked the issue as duplicate of #56

c4-judge commented 9 months ago

alcueca marked the issue as satisfactory

c4-judge commented 8 months ago

alcueca marked the issue as unsatisfactory: Invalid