Insufficient Gas Griefing

Insufficient gas griefing can be done on contracts which accept data and use it in a sub-call on another contract. This method is often used in multisignature wallets as well as transaction relayers. If the sub-call fails, either the whole transaction is reverted, or execution is continued.

Let's consider a simple relayer contract as an example. As shown below, the relayer contract allows someone to make and sign a transaction, without having to execute the transaction. Often this is used when a user can't pay for the gas associated with the transaction.

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);
    }
}

The user who executes the transaction, the 'forwarder', can effectively censor transactions by using just enough gas so that the transaction executes, but not enough gas for the sub-call to succeed.

There are two ways this could be prevented. The first solution would be to only allow trusted users to relay transactions. The other solution is to require that the forwarder provides enough gas, as seen below.

// contract called by Relayer
contract Executor {
    function execute(bytes _data, uint _gasLimit) {
        require(gasleft() >= _gasLimit);
        ...
    }
}

Sources

  • https://consensys.github.io/smart-contract-best-practices/attacks/griefing/
  • https://ethereum.stackexchange.com/questions/62829/what-does-griefing-mean
  • https://ethereum.stackexchange.com/questions/73261/griefing-attacks-are-they-profitable-for-the-attacker
  • https://en.wikipedia.org/wiki/Griefer

Reentrancy

Reentrancy is an attack that can occur when a bug in a contract may allow a malicious contract to reenter the contract unexpectedly during execution of the original function. This can be used to drain funds from a smart contract if used maliciously. Reentrancy is likely the single most impactful vulnerability in terms of total loss of funds by smart contract hacks, and should be considered accordingly. List of reentrancy attacks

External calls

Reentrancy can be executed by the availability of an external call to an attacker controlled contract. External calls allow for the callee to execute arbitrary code. The existence of an external call may not always be obvious, so it's important to be aware of any way in which an external call may be executed in your smart contracts.

ETH transfers

When Ether is transferred to a contract address, it will trigger the receive or fallback function, as implemented in the contract. An attacker can write any arbitrary logic into the fallback method, such that anytime the contract receives a transfer, that logic is executed.

safeMint

One example of a hard to spot external call is OpenZeppelin's ERC721._safeMint & ERC721._safeTransfer functions.

/**
  * @dev Same as {xref-ERC721-_safeMint-address-uint256-}[`_safeMint`], with an additional `data` parameter which is
  * forwarded in {IERC721Receiver-onERC721Received} to contract recipients.
  */
function _safeMint(
    address to,
    uint256 tokenId,
    bytes memory _data
) internal virtual {
    _mint(to, tokenId);
    require(
        _checkOnERC721Received(address(0), to, tokenId, _data),
        "ERC721: transfer to non ERC721Receiver implementer"
    );
}

The function is titled _safeMint because it prevents tokens from being unintentionally minted to a contract by checking first whether that contract has implemented ERC721Receiver, i.e. marking itself as a willing recipient of NFTs. This all seems fine, but _checkOnERC721Received is an external call to the receiving contract, allowing arbitrary execution.

Single function reentrancy

A single function reentrancy attack occurs when a vulnerable function is the same function that an attacker is trying to recursively call.

// UNSECURE
function withdraw() external {
    uint256 amount = balances[msg.sender];
    (bool success,) = msg.sender.call{value: balances[msg.sender]}("");
    require(success);
    balances[msg.sender] = 0;
}

Here we can see that the balance is only modified after the funds have been transferred. This can allow a hacker to call the function many times before the balance is set to 0, effectively draining the smart contract.

Cross-function reentrancy

A cross-function reentrancy attack is a more complex version of the same process. Cross-function reentrancy occurs when a vulnerable function shares state with a function that an attacker can exploit.

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

function withdraw() external {
  uint256 amount = balances[msg.sender];
  (bool success,) = msg.sender.call{value: balances[msg.sender]}("");
  require(success);
  balances[msg.sender] = 0;
}

In this example, a hacker can exploit this contract by having a fallback function call transfer() to transfer spent funds before the balance is set to 0 in the withdraw() function.

Read-only Reentrancy

Read-only reentrancy is a novel attack vector in which instead of reentering into the same contract in which state changes have yet to be made, an attacker reenters into another contract which reads from the state of the original contract.

// UNSECURE
contract A {
	// Has a reentrancy guard to prevent reentrancy
	// but makes state change only after external call to sender
	function withdraw() external nonReentrant {
		uint256 amount = balances[msg.sender];
		(bool success,) = msg.sender.call{value: balances[msg.sender]}("");
		require(success);
		balances[msg.sender] = 0;
	}
}

contract B {
	// Allows sender to claim equivalent B tokens for A tokens they hold
	function claim() external nonReentrant {
		require(!claimed[msg.sender]);
		balances[msg.sender] = A.balances[msg.sender];
		claimed[msg.sender] = true;
	}
}

As we can see in the above example, although both functions have a nonReentrant modifier, it is still possible for an attacker to call B.claim during the callback in A.withdraw, and since the attackers balance was not yet updated, execution succeeds.

Reentrancy prevention

The simplest reentrancy prevention mechanism is to use a ReentrancyGuard, which allows you to add a modifier, e.g. nonReentrant, to functions which may otherwise be vulnerable. Although effective against most forms of reentrancy, it's important to understand how read-only reentrancy may be used to get around this and to always use the checks-effects-interactions pattern.

For optimum security, use the checks-effects-interactions pattern. This is a simple rule of thumb for ordering smart contract functions.

The function should begin with checks, e.g. require and assert statements.

Next, the effects of the contract should be performed, i.e. state modifications.

Finally, we can perform interactions with other smart contracts, e.g. external function calls.

This structure is effective against reentrancy because when an attacker reenters the function, the state changes have already been made. For example:

function withdraw() external {
  uint256 amount = balances[msg.sender];
  balances[msg.sender] = 0;
  (bool success,) = msg.sender.call{value: balances[msg.sender]}("");
  require(success);
}

Since the balance is set to 0 before any interactions are performed, if the contract is called recursively, there is nothing to send after the first transaction.

Examples from: https://medium.com/coinmonks/protect-your-solidity-smart-contracts-from-reentrancy-attacks-9972c3af7c21

Sources

  • https://consensys.github.io/smart-contract-best-practices/attacks/reentrancy/
  • https://medium.com/@gus_tavo_guim/reentrancy-attack-on-smart-contracts-how-to-identify-the-exploitable-and-an-example-of-an-attack-4470a2d8dfe4
  • https://medium.com/coinmonks/protect-your-solidity-smart-contracts-from-reentrancy-attacks-9972c3af7c21

Integer Overflow and Underflow

In solidity, integer types have maximum values. For example:

uint8 => 255 uint16 => 65535 uint24 => 16777215 uint256 => (2^256) - 1

Overflow and underflow bugs can occur when you exceed the maximum value (overflow) or when you go below the minimum value (underflow). When you exceed the maximum value, you go back down to zero, and when you go below the minimum value, it brings you back up to the maximum value.

Since smaller integer types like: uint8, uint16, etc. have smaller maximum values, it can be easier to cause an overflow, thus they should be used with greater caution.

Older contracts often made use of the SafeMath library, to avoid over/underflows, but in solidity >=v0.8.0, SafeMath logic is built in by default. It's important to consider that regardless of SafeMath logic being used, either built-in or used manually in older contracts, over/underflows still trigger reverts, which may result in denial of service of important functionality or other unexpected effects. Built-in SafeMath logic may also be avoided with unchecked blocks, see docs for more info.

Sources

  • https://consensys.github.io/smart-contract-best-practices/attacks/insecure-arithmetic/

Timestamp Dependence

NOTE: This vulnerability no longer affects Ethereum mainnet as of the Proof of Stake merge. Read more

The timestamp of a block, accessed by block.timestamp or alias now can be manipulated by a miner. There are three considerations you should take into account when using a timestamp to execute a contract function.

Timestamp Manipulation

If a timestamp is used in an attempt to generate randomness, a miner can post a timestamp within 15 seconds of block validation, giving them the ability to set the timestamp as a value that would increase their odds of benefitting from the function.

For example, a lottery application may use the block timestamp to pick a random bidder in a group. A miner may enter the lottery then modify the timestamp to a value that gives them better odds at winning the lottery.

Timestamps should thus not be used to create randomness. See Weak Sources of Randomness for Chain Attributes.

The 15-second Rule

Ethereum's reference specification, the Yellow Paper, doesn't specify a limit as to how much blocks can change in time, it just has to be bigger than the timestamp of it's parent. This being said, popular protocol implementations reject blocks with timestamps greater than 15 seconds in the future, so as long as your time-dependent event can safely vary by 15 seconds, it may be safe to use a block timestamp.

Don't use block.number as a timestamp

You can estimate the time difference between events using block.number and the average block time, but block times may change and break the functionality, so it's best to avoid this use.

Sources

  • https://consensys.github.io/smart-contract-best-practices/attacks/timestamp-dependence/
  • https://consensys.github.io/smart-contract-best-practices/development-recommendations/solidity-specific/timestamp-dependence/

Authorization Through tx.origin

tx.origin is a global variable in Solidity which returns the address that sent a transaction. It's important that you never use tx.origin for authorization since another contract can use a fallback function to call your contract and gain authorization since the authorized address is stored in tx.origin. Consider this example:

pragma solidity >=0.5.0 <0.7.0;

// THIS CONTRACT CONTAINS A BUG - DO NOT USE
contract TxUserWallet {
    address owner;

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

    function transferTo(address payable dest, uint amount) public {
        require(tx.origin == owner);
        dest.transfer(amount);
    }
}

Here we can see that the TxUserWallet contract authorizes the transferTo() function with tx.origin. 

pragma solidity >=0.5.0 <0.7.0;

interface TxUserWallet {
    function transferTo(address payable dest, uint amount) external;
}

contract TxAttackWallet {
    address payable owner;

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

    function() external {
        TxUserWallet(msg.sender).transferTo(owner, msg.sender.balance);
    }
}

Now if someone were to trick you into sending ether to the TxAttackWallet contract address, they can steal your funds by checking tx.origin to find the address that sent the transaction.

To prevent this kind of attack, use msg.sender for authorization.

Examples from: https://solidity.readthedocs.io/en/develop/security-considerations.html#tx-origin

Sources

  • https://swcregistry.io/docs/SWC-115
  • https://solidity.readthedocs.io/en/develop/security-considerations.html#tx-origin
  • https://consensys.github.io/smart-contract-best-practices/development-recommendations/solidity-specific/tx-origin/
  • https://github.com/sigp/solidity-security-blog#tx-origin

Floating Pragma

It is considered best practice to pick one compiler version and stick with it. With a floating pragma, contracts may accidentally be deployed using an outdated or problematic compiler version which can cause bugs, putting your smart contract's security in jeopardy. For open-source projects, the pragma also tells developers which version to use, should they deploy your contract. The chosen compiler version should be thoroughly tested and considered for known bugs.

The exception in which it is acceptable to use a floating pragma, is in the case of libraries and packages. Otherwise, developers would need to manually update the pragma to compile locally.

Sources

  • https://swcregistry.io/docs/SWC-103
  • https://consensys.github.io/smart-contract-best-practices/development-recommendations/solidity-specific/locking-pragmas/

Outdated Compiler Version

Developers often find bugs and vulnerabilities in existing software and make patches. For this reason, it's important to use the most recent compiler version possible. See bugs from past compiler versions here.

Sources

  • https://swcregistry.io/docs/SWC-102
  • https://github.com/ethereum/solidity/releases
  • https://etherscan.io/solcbuginfo
  • https://solidity.readthedocs.io/en/latest/bugs.html

Unsafe Low-Level Call

In Solidity, you can either use low-level calls such as: address.call(), address.callcode(), address.delegatecall(), and address.send(); or you can use contract calls such as: ExternalContract.doSomething().

Low-level calls can be a good way to efficiently or arbitrarily make contract calls. However, it's important to be aware of the caveats it possesses.

Unchecked call return value

Low-level calls will never throw an exception, instead they will return false if they encounter an exception, whereas contract calls will automatically throw. Thus if the return value of a low-level call is not checked, the execution may resume even if the function call throws an error. This can lead to unexpected behaviour and break the program logic. A failed call can even be caused intentionally by an attacker, who may be able to further exploit the application.

In the case that you use low-level calls, be sure to check the return value to handle possible failed calls, e.g.:

// Simple transfer of 1 ether
(bool success,) = to.call{value: 1 ether}("");
// Revert if unsuccessful
require(success);

Successful call to non-existent contract

As noted in the Solidity docs: "Due to the fact that the EVM considers a call to a non-existing contract to always succeed, Solidity uses the extcodesize opcode to check that the contract that is about to be called actually exists (it contains code) and causes an exception if it does not. This check is skipped if the return data will be decoded after the call and thus the ABI decoder will catch the case of a non-existing contract.

Note that this check is not performed in case of low-level calls which operate on addresses rather than contract instances."

It's imperative that we do not simply assume that a contract to be called via a low-level call actually exists, since if it doesn't our logic will proceed even though our external call effectively failed. This can lead to loss of funds and/or an invalid contract state. Instead, we must verify that the contract being called exists, either immediately before being called with an extcodesize check, or by verifying during contract deployment and using a constant/immutable value if the contract can be fully trusted.

// Verify address is a contract
require(to.code.length > 0);
// Simple transfer of 1 ether
(bool success,) = to.call{value: 1 ether}("");
// Revert if unsuccessful
require(success);

Sources

  • https://swcregistry.io/docs/SWC-104
  • https://consensys.github.io/smart-contract-best-practices/development-recommendations/general/external-calls/

Uninitialized Storage Pointer

Data is stored in the EVM as either storage, memory, or calldata. It is important that they are well understood and correctly initialized. Incorrectly initializing data storage pointers, or simply leaving them uninitialized, can lead to contract vulnerabilities.

As of solidity 0.5.0, uninitialized storage pointers are no longer an issue since contracts with uninitialized storage pointers will no longer compile. This being said, it's still important to understand what storage pointers you should be using in certain situations.

Sources

  • https://swcregistry.io/docs/SWC-109
  • https://github.com/sigp/solidity-security-blog#storage
  • https://solidity.readthedocs.io/en/latest/types.html#data-location
  • https://solidity.readthedocs.io/en/latest/miscellaneous.html#layout-of-state-variables-in-storage
  • https://solidity.readthedocs.io/en/latest/miscellaneous.html#layout-in-memory

Assert Violation

In Solidity 0.4.10, the following functions were created: assert(), require(), and revert(). Here we'll discuss the assert function and how to use it.

Formally said, the assert() funtion is meant to assert invariants; informally said, assert() is an overly assertive bodyguard that protects your contract, but steals your gas in the process. Properly functioning contracts should never reach a failing assert statement. If you've reached a failing assert statement, you've either improperly used assert(), or there is a bug in your contract that puts it in an invalid state.

If the condition checked in the assert() is not actually an invariant, it's suggested that you replace it with a require() statement.

Sources

  • https://swcregistry.io/docs/SWC-110
  • https://media.consensys.net/when-to-use-revert-assert-and-require-in-solidity-61fb2c0e5a57

Use of Deprecated Functions

As time goes by, functions and operators in Solidity are deprecated and often replaced. It's important to not use deprecated functions, as it can lead to unexpected effects and compilation errors.

Here is a non-exhaustive list of deprecated functions and alternatives. Many alternatives are simply aliases, and won't break current behaviour if used as a replacement for its deprecated counterpart.

DeprecatedAlternatives
suicide(address)selfdestruct(address)
block.blockhash(uint)blockhash(uint)
sha3(...)keccak256(...)
callcode(...)delegatecall(...)
throwrevert()
msg.gasgasleft
constantview
varcorresponding type name

Sources

  • https://swcregistry.io/docs/SWC-111
  • https://github.com/ethereum/solidity/releases

Delegatecall to Untrusted Callee

Delegatecall is a special variant of a message call. It is almost identical to a regular message call except the target address is executed in the context of the calling contract and msg.sender and msg.value remain the same. Essentially, delegatecall delegates other contracts to modify the calling contract's storage.

Since delegatecall gives so much control over a contract, it's very important to only use this with trusted contracts such as your own. If the target address comes from user input, be sure to verify that it is a trusted contract.

Sources

  • https://swcregistry.io/docs/SWC-112
  • https://solidity.readthedocs.io/en/latest/introduction-to-smart-contracts.html#delegatecall-callcode-and-libraries
  • https://blog.sigmaprime.io/solidity-security.html#delegatecall
  • https://ethereum.stackexchange.com/questions/3667/difference-between-call-callcode-and-delegatecall

Signature Malleability

It's generally assumed that a valid signature cannot be modified without the private key and remain valid. However, it is possible to modify and signature and maintain validity. One example of a system which is vulnerable to signature malleability is one in which validation as to whether an action can be executed is determined based on whether the signature has been previously used.

// UNSECURE
require(!signatureUsed[signature]);

// Validate signer and perform state modifying logic
...

signatureUsed[signature] = true;

In the above example, we can see that the signature is saved in a signatureUsed mapping after execution and validated to not exist in that mapping before execution. The problem with this is that if the signature can be modified while maintaining valididty, the transaction can be repeated by an attacker.

How it works

To understand how signature malleability works, we first need to understand a bit about elliptic curve cryptography.

An elliptic curve consists of all the points that satisfy an equation of the form:

$y^2 = x^3 + ax + b$

where

$4a^3 + 27b^2 \not= 0$ (to avoid singular points)

Some examples:

Elliptic Curves

Note that the curves are always symmetrical about the x-axis

The curve used by Ethereum is secp256k1, which looks like:

secp256k1

Now that we understand the basics of elliptic curve cryptography, we can dig into how signature malleability actually works on Ethereum.

Ethereum uses ECDSA as it's signature scheme. ECDSA signatures consist of a pair of numbers, $(r, s)$, with an integer order $n$. As a result of the x-axis symmetry, if $(r, s)$ is a valid signature, then so is $(r, -s$ mod $n)$.

It's possible to calculate this complementary signature without knowing the private key used to produce it in the first place, which gives an attacker the ability to produce a second valid signature.

Mitigation

To avoid this issue, it's imperative to recognize that validating that a signature is not reused is insufficient in enforcing that the transaction is not replayed.

Sources

  • https://swcregistry.io/docs/SWC-117
  • https://eklitzke.org/bitcoin-transaction-malleability
  • https://hackernoon.com/what-is-the-math-behind-elliptic-curve-cryptography-f61b25253da3

Incorrect Constructor Name

Before Solidity 0.4.22, the only way to define a constructor was by creating a function with the contract name. In some cases this was problematic. For example, if a smart contract is re-used with a different name but the constructor function isn't also changed it simply becomes a regular, callable function. Similarly, it's possible for an attacker to create a contract with which a function appears to be the constructor but actually has one character replaced with a similar looking character, e.g. replacing an "l" with a "1", allowing logic to be executed when it's only expected to be executed during contract creation.

Now with modern versions of Solidity, the constructor is defined with the constructor keyword, effectively deprecating this vulnerability. Thus the solution to this problem is simply to use modern Solidity compiler versions.

Sources

  • https://swcregistry.io/docs/SWC-118
  • https://blog.sigmaprime.io/solidity-security.html#constructors

Shadowing State Variables

It is possible to use the same variable twice in Solidity, but it can lead to unintended side effects. This is especially difficult regarding working with multiple contracts. Take the following example:

contract SuperContract {
  uint a = 1;
}

contract SubContract is SuperContract {
  uint a = 2;
}

Here we can see that SubContract inherits SuperContract and the variable a is defined twice with different values. Now say we use a to perform some function in SubContract, functionality inherited from SuperContract will no longer work since the value of a has been modified.

To avoid this vulnerability, it's important we check the entire smart contract system for ambiguities. It's also important to check for compiler warnings, as they can flag these ambiguities so long as they're in the smart contract.

Sources

  • https://swcregistry.io/docs/SWC-119
  • https://github.com/ethereum/solidity/issues/2563
  • https://github.com/ethereum/solidity/issues/973

Weak Sources of Randomness from Chain Attributes

Using chain attributes for randomness, e.g.: block.timestamp, blockhash, and block.difficulty can seem like a good idea since they often produce pseudo-random values. The problem however, is that Ethereum is entirely deterministic and all available on-chain data is public. Chain attributes can either be predicted or manipulated, and should thus never be used for random number generation.

A common solution is to use an oracle solution such as Chainlink VRF.

Sources

  • https://swcregistry.io/docs/SWC-120
  • https://ethereum.stackexchange.com/questions/419/when-can-blockhash-be-safely-used-for-a-random-number-when-would-it-be-unsafe
  • https://ethereum.stackexchange.com/questions/191/how-can-i-securely-generate-a-random-number-in-my-smart-contract
  • https://fravoll.github.io/solidity-patterns/randomness.html

Missing Protection against Signature Replay Attacks

Sometimes in smart contracts it is necessary to perform signature verification to improve usability and gas cost. However, consideration needs to be taken when implementing signature verification. To protect against Signature Replay Attacks, the contract should only be allowing new hashes to be processed. This prevents malicious users from replaying another users signature multiple times.

To be extra safe with signature verification, follow these recommendations:

  • Store every message hash processed by the contract, then check messages hashes against the existing ones before executing the function.
  • Include the address of the contract in the hash to ensure that the message is only used in a single contract.
  • Never generate the message hash including the signature. See Signature Malleability

Sources

  • https://swcregistry.io/docs/SWC-121
  • https://medium.com/cypher-core/replay-attack-vulnerability-in-ethereum-smart-contracts-introduced-by-transferproxy-124bf3694e25

Requirement Violation

The require() method is meant to validate conditions, such as inputs or contract state variables, or to validate return values from external contract calls. For validating external calls, inputs can be provided by callers, or they can be returned by callees. In the case that an input violation has occured by the return value of a callee, likely one of two things has gone wrong:

  • There is a bug in the contract that provided the input.
  • The requirement condition is too strong.

To solve this issue, first consider whether the requirement condition is too strong. If necessary, weaken it to allow any valid external input. If the problem isn't the requirement condition, there must be a bug in the contract providing external input. Ensure that this contract is not providing invalid inputs.

Sources

  • https://swcregistry.io/docs/SWC-123
  • https://media.consensys.net/when-to-use-revert-assert-and-require-in-solidity-61fb2c0e5a57

Write to Arbitrary Storage Location

Only authorized addresses should have access to write to sensitive storage locations. If there isn't proper authorization checks throughout the contract, a malicious user may be able to overwrite sensitive data. However, even if there are authorization checks for writing to sensitive data, an attacker may still be able to overwrite the sensitive data via insensitive data. This could give an attacker access to overwrite important variables such as the contract owner.

To prevent this from occuring, we not only want to protect sensitive data stores with authorization requirements, but we also want to ensure that writes to one data structure cannot inadvertently overwrite entries of another data structure.

For an example, try Ethernaut - Alien Codex. If it's too hard, see this walkthrough (SPOILER).

Sources

  • https://swcregistry.io/docs/SWC-124
  • https://github.com/Arachnid/uscc/tree/master/submissions-2017/doughoyte

Incorrect Inheritance Order

In Solidity, it is possible to inherit from multiple sources, which if not properly understood can introduce ambiguity. This ambiguity is known as the Diamond Problem, wherein if two base contracts have the same functon, which one should be prioritized? Luckily, Solidity handles this problem gracefully, that is as long as the developer understands the solution.

The solution Solidity provides to the Diamond Problem is by using reverse C3 linearization. This means that it will linearize the inheritance from right to left, so the order of inheritance matters. It is suggested to start with more general contracts and end with more specific contracts to avoid problems.

Sources

  • https://consensys.github.io/smart-contract-best-practices/development-recommendations/solidity-specific/complex-inheritance/
  • https://solidity.readthedocs.io/en/v0.4.25/contracts.html#multiple-inheritance-and-linearization
  • https://pdaian.com/blog/solidity-anti-patterns-fun-with-inheritance-dag-abuse/

Presence of Unused Variables

Although it is allowed, it is best practice to avoid unused variables. Unused variables can lead to a few different problems:

  • Increase in computations (unnecessary gas consumption)
  • Indication of bugs or malformed data structures
  • Decreased code readability

It is highly recommended to remove all unused variables from a code base.

Sources

  • https://swcregistry.io/docs/SWC-131
  • https://github.com/ethereum/solidity/issues/718
  • https://github.com/ethereum/solidity/issues/2563

Unencrypted Private Data On-Chain

Ethereum smart contract code, storage, and any data transacted on-chain can always be read. Treat it as such. Even if your code is not verified on Etherscan, attackers can still decompile or check transactions to and from it to analyze it. For this reason, it's imperative that private data is never stored on-chain unencrypted.

Inadherence to Standards

In terms of smart contract development, it's important to follow standards. Standards are set to prevent vulnerabilities, and ignoring them can lead to unexpected effects.

Take for example binance's original BNB token. It was marketed as an ERC20 token, but it was later pointed out that it wasn't actually ERC20 compliant for a few reasons:

  • It prevented sending to 0x0
  • It blocked transfers of 0 value
  • It didn't return true or false for success or fail

The main cause for concern with this improper implementation is that if it is used with a smart contract that expects an ERC-20 token, it will behave in unexpected ways. It could even get locked in the contract forever.

Although standards aren't always perfect, and may someday become antiquated, they foster proper expectations to provide for secure smart contracts.

Suggested by: RobertMCForster

Sources

  • https://finance.yahoo.com/news/bnb-really-erc-20-token-160013314.html
  • https://blog.goodaudience.com/binance-isnt-erc-20-7645909069a4

Asserting contract from Code Size

A common method for asserting whether a sender is a contract or EOA has been to check the code size of the sender. This check asserts that if the sender has a code size > 0 that it must be a contract and if not then it must be an EOA. For example:

function mint(uint256 amount) public {
  if (msg.sender.code.length != 0) revert CallerNotEOA();
}

However, as noted in the Ethereum Yellow Paper, "During initialization code execution, EXTCODESIZE on the address should return zero, which is the length of the code of the account while CODESIZE should return the length of the initialization".

This repo shows how we may exploit this logic by simply calling during creation of a new contract.

As we can see, it's important that we recognize that although we may be certain that an account with a non-zero codesize is a contract, we can't be certain that an account with a zero codesize is not a contract.

Sources

  • https://ethereum.github.io/yellowpaper/paper.pdf
  • https://github.com/0xKitsune/Ghost-Contract

Transaction-Ordering Dependence

Transactions on Ethereum are grouped together in blocks which are processed on a semi-regular interval, 12 seconds. Before transactions are placed in blocks, they are broadcasted to the mempool where block builders can then proceed to place them as is economically optimal. What's important to understand here is that the mempool is public and thus anyone can see transactions before they're executed, giving them the power to frontrun by placing their own transaction executing the same, or a similar, action with a higher gas price.

Frontrunning has become prevalent as a result of generalized frontrunning bots becoming more and more common. These generalized frontrunners work by observing the mempool for profitable, replicable transactions which they can replace for their own benefit. Ethereum is a Dark Forest.

One solution to transaction-ordering dependence is to use a commit-reveal scheme in the case of information being submitted on-chain. This works by having the submitter send in a hash of the information, storing that on-chain along with the user address so that they may later reveal the answer along with the salt to prove that they were indeed correct. Another solution is to simply use a private mempool such as Flashbots.

Sources

  • https://consensys.github.io/smart-contract-best-practices/known_attacks/#front-running-aka-transaction-ordering-dependence
  • https://users.encs.concordia.ca/~clark/papers/2019_wtsc_front.pdf
  • https://swcregistry.io/docs/SWC-114

DoS with Block Gas Limit

One of the primary benefits of a block gas limit is that it prevents attackers from creating an infinite transaction loop. If the gas usage of a transaction exceeds this limit, the transaction will fail. However, along with this benefit comes a side effect which is important to understand.

Unbounded Operations

An example in which the block gas limit can be an issue is in executing logic in an unbounded loop. Even without any malicious intent, this can easily go wrong. Just by e.g., having too large an array of users to send funds to can exceed the gas limit and prevent the transaction from ever succeeding, potentially permanently locking up funds.

This situation can also lead to an attack. Say a bad actor decides to create a significant amount of addresses, with each address being paid a small amount of funds from the smart contract. If done effectively, the transaction can be blocked indefinitely, possibly even preventing further transactions from going through.

An effective solution to this problem would be to use a pull payment system over the above push payment system. To do this, separate each payment into it's own transaction, and have the recipient call the function.

If, for some reason, you really need to loop through an array of unspecified length, at least expect it to potentially take multiple blocks, and allow it to be performed in multiple transactions - as seen in this example:

struct Payee {
    address addr;
    uint256 value;
}

Payee[] payees;
uint256 nextPayeeIndex;

function payOut() {
    uint256 i = nextPayeeIndex;
    while (i < payees.length && msg.gas > 200000) {
      payees[i].addr.send(payees[i].value);
      i++;
    }
    nextPayeeIndex = i;
}

Block Stuffing

In some situations, your contract can be attacked with a block gas limit even if you don't loop through an array of unspecified length. An attacker can fill several blocks before a transaction can be processed by using a sufficiently high gas price.

This attack is done by issuing several transactions at a very high gas price. If the gas price is high enough, and the transactions consume enough gas, they can fill entire blocks and prevent other transactions from being processed.

Ethereum transactions require the sender to pay gas to disincentivize spam attacks, but in some situations, there can be enough incentive to go through with such an attack. For example, a block stuffing attack was used on a gambling Dapp, Fomo3D. The app had a countdown timer, and users could win a jackpot by being the last to purchase a key, except everytime a user bought a key, the timer would be extended. An attacker bought a key then stuffed the next 13 blocks in a row so they could win the jackpot.

To prevent such attacks from occuring, it's important to carefully consider whether it's safe to incorporate time-based actions in your application.

Example from: https://consensys.github.io/smart-contract-best-practices/attacks/denial-of-service/

Sources

  • https://consensys.github.io/smart-contract-best-practices/attacks/denial-of-service/
  • https://github.com/ethereum/wiki/wiki/Design-Rationale#gas-and-fees

DoS with (Unexpected) revert

A DoS (Denial of Service) may be caused when logic is unable to be executed as a result of an unexpected revert. This can happen for a number of reasons and it's important to consider all the ways in which your logic may revert. The examples listed below are non-exhaustive.

Reverting funds transfer

DoS (Denial of Service) attacks can occur in functions when you try to send funds to a user and the functionality relies on that fund transfer being successful.

This can be problematic in the case that the funds are sent to a smart contract created by a bad actor, since they can simply create a fallback function that reverts all payments.

For example:

// INSECURE
contract Auction {
    address currentLeader;
    uint highestBid;

    function bid() payable {
        require(msg.value > highestBid);

        require(currentLeader.send(highestBid)); // Refund the old leader, if it fails then revert

        currentLeader = msg.sender;
        highestBid = msg.value;
    }
}

As you can see in this example, if an attacker bids from a smart contract with a fallback function reverting all payments, they can never be refunded, and thus no one can ever make a higher bid.

This can also be problematic without an attacker present. For example, you may want to pay an array of users by iterating through the array, and of course you would want to make sure each user is properly paid. The problem here is that if one payment fails, the funtion is reverted and no one is paid. 

address[] private refundAddresses;
mapping (address => uint) public refunds;

// bad
function refundAll() public {
    for(uint x; x < refundAddresses.length; x++) { // arbitrary length iteration based on how many addresses participated
        require(refundAddresses[x].send(refunds[refundAddresses[x]])) // doubly bad, now a single failure on send will hold up all funds
    }
}

An effective solution to this problem would be to use a pull payment system over the above push payment system. To do this, separate each payment into it's own transaction, and have the recipient call the function.

contract auction {
    address highestBidder;
    uint highestBid;
    mapping(address => uint) refunds;

    function bid() payable external {
        require(msg.value >= highestBid);

        if (highestBidder != address(0)) {
            refunds[highestBidder] += highestBid; // record the refund that this user can claim
        }

        highestBidder = msg.sender;
        highestBid = msg.value;
    }

    function withdrawRefund() external {
        uint refund = refunds[msg.sender];
        refunds[msg.sender] = 0;
        (bool success, ) = msg.sender.call.value(refund)("");
        require(success);
    }
}

Examples from: https://consensys.github.io/smart-contract-best-practices/attacks/denial-of-service/ https://consensys.github.io/smart-contract-best-practices/development-recommendations/general/external-calls/

Over/Underflow

Prior to SafeMath usage, whether built-in in solidity >=0.8.0 or using a library, over/underflows would result in rolling over to the minimum/maxium value. Now that checked math is commonplace, it's important to recognize that the effect of checked under/overflows is a revert, which may DoS important logic.

Regardless of usage of checked math, it's necessary to ensure that any valid input will not result in an over/underflow. Take extra care when working with smaller integers e.g. int8/uint8, int16/uint16, int24/uint24, etc..

Unexpected Balance

It's important to take caution in enforcing expected contract balances of tokens or Ether as those balances may be increased by an attacker to cause an unexpected revert. This is easily possible with ERC20 tokens by simply transferring to the contract, but is also possible with Ether by Forcibly sending Ether to a contract.

Consider, for example, a contract which expects the Ether balance to be 0 for the first deposit to allow for custom accounting logic. An attacker may forcibly send Ether to the contract before the first deposit, causing all deposits to revert.

Sources

  • https://consensys.github.io/smart-contract-best-practices/attacks/denial-of-service/
  • https://consensys.github.io/smart-contract-best-practices/development-recommendations/general/external-calls/

Unexpected ecrecover Null Address

ecrecover is a precompiled built-in cryptographic function which recovers an address associated with the public key from an elliptic curve signature or returns zero on error. The parameters corresponding to the signature are r, s & v.

As noted above, ecrecover will return zero on error. It's possible to do this deterministically by setting v as any positive number other than 27 or 28.

ecrecover is often used to verify that the signer is an authorized account. The problem with this is that uninitialized or renounced authorization logic often sets the owner/admin address as address(0), the same value which may be deterministically returned by ecrecover. This means that an unsecure contract may allow an attacker to spoof an authorized-only method into executing as though the authorized account is the signer.

// UNSECURE
function setOwner(bytes32 newOwner, uint8 v, bytes32 r, bytes32 s) external {
	address signer = ecrecover(newOwner, v, r, s);
	require(signer == owner);
	owner = address(newOwner);
}

The above method is intended to only set a new owner if a valid signature from the existing owner is provided. However, as we know, if we set v to any value other than 27 or 28, the signer will be the null address and if the current owner is uninitialized or renounced, the require statement will succeed allowing an attacker to set themselves as owner.

We can mitigate this issue by reverting if the recovered signer address is null, e.g.:

function setOwner(bytes32 newOwner, uint8 v, bytes32 r, bytes32 s) external {
	address signer = ecrecover(newOwnerHash, v, r, s);
	require(signer == owner && signer != address(0));
	owner = address(newOwner);
}

Sources

  • https://docs.soliditylang.org/en/latest/units-and-global-variables.html#mathematical-and-cryptographic-functions
  • https://ethereum.stackexchange.com/a/69329

Default Visibility

Visibility specifiers are used to determine where a function or variable can be accessed from. As explained in the solidity docs:

  • public: visible externally and internally (creates a getter function for storage/state variables)
  • private: only visible in the current contract
  • external: only visible externally (only for functions) - i.e. can only be message-called (via this.func)
  • internal: only visible internally

It's important to note that the default visibility is public, allowing access externally or internally by any contract or EOA. We can see how this may be a problem if a method is intended to only be accessible internally but is missing a visibility specifier.

Modern compilers should catch missing function visibility specifiers, but will generally allow missing state variable visibility specifiers. Regardless, it's important to be aware of the possible interactions which may occur as a result of default visibility specifiers for both functions and state variables.

Sources

  • https://swcregistry.io/docs/SWC-100
  • https://swcregistry.io/docs/SWC-108
  • https://consensys.github.io/smart-contract-best-practices/development-recommendations/solidity-specific/visibility/
  • https://github.com/sigp/solidity-security-blog#visibility

Insufficient Access Control

Access control is often imperative in management and ownership of smart contracts. It's important to consider ways in which access control may be circumvented, or insufficiently implemented and the corresponding consequences.

Off-By-One

Off-by-one errors are a common mistake made by programmers in which the intended boundaries are incorrect by only one, though these errors may seem insignificant, the effect can easily be quite severe.

Array lengths

Properly determining intended array lengths is a common source of off-by-one errors. Particularly since 0-indexing means the final value in an array is array.length - 1.

Consider for example a function intended to loop over a list of recipients to transfer funds to each user, but the loop length is incorrectly set.

// Incorrectly sets upper bound to users.length - 1
// Final user in array doesn't receive token transfer
for (uint256 i; i < users.length - 1; ++i) {
	token.transfer(users[i], 1 ether);
}

Incorrect comparison operator

It's common for comparison operators to be off by one when, e.g. > should be used in place of >=. This is especially common when the logic includes some kind of negation, leading to mental friction in deciphering the intended vs implemented bounds.

Consider for example a Defi protocol with liquidation logic documented to liquidate a user only if their collateralization ratio is below 1e18.

// Incorrectly liquidates if collateralizationRatio is == 1 ether
if (collateralizationRatio > 1 ether) {
	...
} else {
	liquidate();
}

Sources

  • https://github.com/OpenCoreCH/smart-contract-auditing-heuristics#off-by-one-errors

Lack of Precision

In Solidity, there are a limited variety of number types. Differently from many programming languages, floating point numbers are unsupported. Fixed point numbers are partially supported, but cannot be assigned to or from. The primary number type in Solidity are integers, of which resulting values of calculations are always rounded down.

Since division often results in a remainder, performing division with integers generally requires a lack of precision to some degree. To see how a lack of precision may cause a serious flaw, consider the following example in which we charge a fee for early withdrawals denominated in the number of days early that the withdrawal is made:

uint256 daysEarly = withdrawalsOpenTimestamp - block.timestamp / 1 days
uint256 fee = amount / daysEarly * dailyFee

The problem with this is that in the case that a user withdraws 1.99 days early, since 1.99 will round down to 1, the user only pays about half the intended fee.

In general, we should ensure that numerators are sufficiently larger than denominators to avoid precision errors. A common solution to this problem is to use fixed point logic, i.e. raising integers to a sufficient number of decimals such that the lack of precision has minimal effect on the contract logic. A good rule of thumb is to raise numbers to 1e18 (commonly referred to as WAD).