S-20200214

Security Intermediate February 14, 2020

Back to All Tasks

Problem Statement

You are tasked with developing a simple Ethereum smart contract for a decentralized charity fund. The contract allows users to donate Ether, and the owner of the contract can withdraw funds to help those in need. However, your initial implementation has a critical security vulnerability that could allow an attacker to drain all funds from the contract through a reentrancy attack.

Your task is to identify the vulnerability in the provided code snippet, explain how it works, and rewrite the smart contract to fix this issue. Additionally, implement a function that allows the owner to check the balance of the contract at any time without using low-level calls.

Concepts

  • Reentrancy Attack
  • Smart Contract Security
  • Solidity Best Practices

Constraints

  • Use Solidity version ^0.8.0
  • Do not use third-party libraries for this task

Security Notes

  • Always be cautious with external calls in your smart contracts, as they can lead to reentrancy vulnerabilities.
  • Using modifiers like `nonReentrant` from the OpenZeppelin library is a common practice to prevent such attacks, but for this exercise, you should manually refactor the code to avoid reentrancy.
  • Regularly audit and test your smart contracts in various scenarios to identify potential security flaws.

Solutions

Python Solution

pragma solidity ^0.8.0;

contract CharityFund {
    address public owner;
    uint256 public totalDonations;
    mapping(address => uint256) public donations;

    // Constructor to set the contract owner
    constructor() {
        owner = msg.sender;
    }

    // Function to donate Ether to the charity fund
    function donate() external payable {
        require(msg.value > 0, "Donation amount must be greater than zero");
        donations[msg.sender] += msg.value;
        totalDonations += msg.value;
    }

    // Function to check the contract balance
    function getContractBalance() external view returns (uint256) {
        return address(this).balance;
    }

    // Refactored withdraw function to prevent reentrancy attacks
    function withdraw(uint256 _amount) external {
        require(msg.sender == owner, "Only the owner can withdraw funds");
        require(_amount <= totalDonations, "Withdrawal amount exceeds total donations");

        // Update state variables before making the external call
        totalDonations -= _amount;
        donations[owner] += _amount;

        // Transfer Ether to the owner
        payable(owner).transfer(_amount);
    }
}

The initial implementation of the CharityFund smart contract had a critical reentrancy vulnerability in the withdraw function. This vulnerability arises when an external call is made before updating the state variables, allowing an attacker to recursively call the vulnerable function and drain the contract's funds.

In this refactored version of the contract, we have moved the state updates (decreasing totalDonations and increasing donations for the owner) before making the Ether transfer. This ensures that even if the external call were to re-enter the withdraw function, the state would already be updated, preventing multiple withdrawals of the same funds.

Additionally, a new function getContractBalance has been added to allow the owner to check the balance of the contract at any time without using low-level calls. This function simply returns the balance of the contract address, which is safe and does not pose any reentrancy risks.

By following these best practices, we can significantly enhance the security of the smart contract and protect it from potential attacks.