code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

Reentrancy concerns with sendValue, error handling in low-level calls, and potential issues with gas costs post EIP-1884. #564

Closed c4-submissions closed 9 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/Address.sol#L64

Vulnerability details

Impact

The sendValue function in the provided code is intended to safely transfer Ether to a given address, but its current implementation poses reentrancy risks. Reentrancy is a common vulnerability in smart contracts where a called contract calls back into the calling contract before the first execution is complete. This can lead to unexpected behavior, including state changes that were supposed to happen only after the transfer of value.

Additionally, the low-level calls (call, staticcall, and delegatecall) used in various functions do not perform any gas stipend checks, which can lead to out-of-gas errors post EIP-1884, where certain opcode costs were increased. This might make contracts unable to receive funds via transfer if they perform operations in their fallback functions that exceed the 2300 gas stipend.

Lastly, the error handling in these low-level calls relies on the return value without explicitly checking for gas usage, which may not be sufficient to prevent out-of-gas errors caused by the callee, potentially leading to failed transactions where the error could have been anticipated and handled more gracefully.

Proof of Concept

As a library contract, direct links to GitHub or screenshots are not applicable here unless the library is part of a specific project. However, you can review the code where the sendValue function is implemented and note the absence of reentrancy guards. Similarly, inspect the functionCall, functionCallWithValue, functionStaticCall, and functionDelegateCall functions for their use of low-level calls without gas checks.

Tools Used

To identify the reentrancy concerns, manual code review can be complemented with tools like:

Slither - a Solidity static analysis framework. Mythril - a security analysis tool for Ethereum smart contracts. For gas cost-related issues, particularly post EIP-1884, one could use:

Gasper - which analyzes gas usage for Ethereum smart contracts. Remix IDE - with its built-in gas profiler.

Recommended Mitigation Steps

Reentrancy Mitigation: Implement a reentrancy guard in the sendValue function. This could be a simple boolean state that gets toggled on function entry and exit, and checked at the start of the function to prevent nested (reentrant) calls.

Gas Stipend Handling: Adjust the gas stipend for low-level calls to ensure that they are compatible with the gas costs post EIP-1884. This can be a fixed amount or dynamically calculated based on the current gas price.

Improved Error Handling: Instead of just relying on the success return value, check the gas left after a low-level call to ensure that the callee has not run out of gas. If it has, handle this explicitly, perhaps by returning a specific error code or taking a predefined recovery action.

By addressing these concerns, the smart contract library's robustness can be significantly improved against common attack vectors and operational hazards.

Assessed type

Reentrancy

c4-pre-sort commented 10 months ago

141345 marked the issue as insufficient quality report

alex-ppg commented 9 months ago

The Warden cites a potential vulnerability in Address which is out-of-scope of the contest.

c4-judge commented 9 months ago

alex-ppg marked the issue as unsatisfactory: Out of scope