Skip to content

Update EtherStore.sol #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
luis-immunefi wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update EtherStore.sol #19

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/EtherStore.sol
View file
Open in desktop

@immunefi-magnus immunefi-magnus Bot Aug 18, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-entrancy and underflow in withdraw enable balance inflation and Ether theft in EtherStore

Description:

TL;DR:
The withdraw function in EtherStore is exposed to both reentrancy and underflow vulnerabilities, allowing an attacker to recursively withdraw funds while inflating their balance.

The vulnerability arises because the withdraw function first sends Ether to msg.sender and only then updates the user's balance to -1, which, due to underflow in an unsigned integer, becomes 2^256 - 1. This mis-sequencing permits reentrant calls, enabling an attacker to repeatedly withdraw funds.

Details

The function retrieves the balance from the balances mapping and checks that it is greater than zero. It then transfers the balance to the caller using a low-level call:

(bool sent, ) = msg.sender.call{value: bal}("");

After the transfer, instead of zeroing out the balance, the contract sets balances[msg.sender] to -1. Since balances is of type uint256, this assignment underflows, converting -1 to 2^256 - 1. Consequently, when the attacker invokes the function, the sequence of an external call before state update allows them to recursively call withdraw several times. This breaks the intended invariant, as the balance becomes artificially inflated and the attacker can drain the contract's funds.

Impact

Exploitation of this vulnerability can lead to complete depletion of the contract's Ether reserves. The attacker can repeatedly call withdraw by taking advantage of the underflowed large balance, effectively compromising the integrity of the contract's accounting and resulting in significant financial loss.

Mitigation Steps:
Update the state by setting balances[msg.sender] to 0 before making the external call to prevent reentrancy.
-Implement a reentrancy guard to restrict recursive calls to the withdraw function.
-Remove any erroneous assignments that lead to underflow, ensuring that balance updates accurately reflect withdrawals.

  • codexa

@immunefi-magnus immunefi-magnus Bot Aug 18, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unchecked underflow in withdraw() grants users maximum balance

Description:

TL;DR:
The withdraw() function in the EtherStore contract sets balances[msg.sender] to -1, which underflows to 2^256-1, granting the caller an effectively unlimited balance.

The withdraw() function improperly assigns -1 to balances[msg.sender] after transferring the user's balance. Since balances is a uint256 mapping, this underflow corrupts the user's balance to the maximum possible value, allowing repeated withdrawals far exceeding the legitimate balance.

Details

In the EtherStore contract, the function withdraw() reads the balance of the caller from the balances mapping and enforces that it is greater than zero. It then transfers the value specified by this balance using a low-level .call(). After successfully transferring the Ether, the function assigns -1 to balances[msg.sender]. However, since balances is declared as a uint256, assigning -1 results in an underflow, causing the value to wrap around to 2^256 - 1. This means that the user now has a balance so large that it can be repeatedly withdrawn, which effectively gives the attacker an unlimited balance. The code snippet illustrating the vulnerability is:

function withdraw() public {
    uint256 bal = balances[msg.sender];
    require(bal > 0);

    (bool sent,) = msg.sender.call{value: bal}("");
    require(sent, "Failed to send Ether");

    balances[msg.sender] = -1; // Underflow occurs here
}

Impact

Exploitation of this vulnerability allows any user with a nonzero balance to repeatedly call the withdraw() function and drain the contract's funds. The underflow in balances[msg.sender] corrupts the accounting mechanism, meaning that even after withdrawing funds, the user’s balance remains effectively unlimited. This can lead to the complete depletion of Ether from the contract, thus having critical financial implications for the protocol.

Mitigation Steps:
Correct the balance update in withdraw() by setting balances[msg.sender] to 0 instead of -1.
-Implement input validation to prevent assignments that result in underflow/overflow.
-Utilize Solidity version 0.8 or later, which has built-in overflow and underflow checks, or incorporate libraries such as SafeMath.
-Reorder state changes and external calls appropriately to avoid known vulnerability patterns.

  • codexa

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ contract EtherStore {
(bool sent,) = msg.sender.call{value: bal}("");
require(sent, "Failed to send Ether");

balances[msg.sender] = 0;
balances[msg.sender] = -1;
}

function getBalance() public view returns (uint256) {
Expand Down