Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update overflow-underflow.md #82

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 100 additions & 15 deletions vulnerabilities/overflow-underflow.md
Original file line number Diff line number Diff line change
@@ -1,73 +1,158 @@
## Integer Overflow and Underflow
In solidity, Integer types have maximum and minimum values. Integer overflow occurs when an integer variable exceeds the maximum value that can be stored in that variable type. Similarly, Integer underflow occurs when an integer variable goes below the minimum value for that variable type. Example: The maximum value ``uint8`` can store is ``255``. Now, when you store ``256`` in ``uint8`` it will overflow and the value will reset to 0. When you store ``257``, the value will be ``1``, ``2`` for ``258`` and so on. Similarly, if you try to store ``-1`` in the uint8 variable the value of the variable will become ``255``, and so on as it will underflow.
In solidity, integer data types have maximum and minimum values. An Integer overflow occurs when an integer exceeds the maximum value that can be stored in that integer type. Similarly, an integer underflow occurs when an integer goes below the minimum value for that integer type.

Example: The maximum value `uint8` can store is ``255``. When you store `256` in `uint8` it will overflow and the value will reset to 0. When you store `257`, the value will be `1`, `2` for `258` and so on. Similarly, if you try to store `-1` in `uint8` the value becomes `255`, and so on as it will underflow.

Some integer types and their min/max values:
| Type | Max | Min |
|----------|-------------|------|
| uint8 | 255 | 0 |
| uint16 | 65535 | 0 |
| uint24 | 16777215 | 0 |
| uint256 | 2^256 - 1 | 0 |
| uint256 | 2<sup>256</sup> - 1 | 0 |

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 great caution.

To prevent over/underflows, the [Safe Math Library](https://github.com/ConsenSysMesh/openzeppelin-solidity/blob/master/contracts/math/SafeMath.sol) is often used by contracts with older versions of Solidity.

Solidity >=v0.8 protects against integer overflows and underflows by default through the use of built-in safe math functions.

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.
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](https://github.com/kadenzipfel/smart-contract-vulnerabilities/blob/master/vulnerabilities/dos-revert.md) of important functionality or other unexpected effects.

To prevent over/underflows, [Safe Math Library](https://github.com/ConsenSysMesh/openzeppelin-solidity/blob/master/contracts/math/SafeMath.sol) is often used by contracts with older versions of Solidity but Solidity >=0.8 protects against integer overflows and underflows through the use of built-in safe math functions. 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](https://github.com/kadenzipfel/smart-contract-vulnerabilities/blob/master/vulnerabilities/dos-revert.md) of important functionality or other unexpected effects. Even after the update of solidity to 0.8, there are scenarios in which the integer overflow and underflow can still occur without the transaction reverting.
Even after the update of solidity to 0.8, there are scenarios in which integer overflows and underflows can still occur without the transaction reverting.

### Typecasting
The most common way in which integer over/underflow is possible when you convert an integer of a larger uint data type to a smaller data type.
The most common way in which integer over/underflow happens is when converting an integer of a larger data type to a smaller data type as seen below:

```solidity
uint256 public a = 258;
uint8 public b = uint8(a); // typecasting uint256 to uint8
```

The above code snippet will overflow and the ``2`` will be stored in the variable ``b`` due to the fact that maximum value in uint8 data type is ``255``. So, it will overflow and reset to ``0`` without reverting.
The above code snippet will overflow and the `2` will be stored in the variable `b` due to the fact that maximum value in `uint8` data type is `255`. So, it will overflow and reset to `0` without reverting.

### Using Shift Operators
Overflow & underflow checks are not performed for shift operations like they are performed for other arithmetic operations. Thus, over/underflows can occur.
Overflow & underflow checks are not performed for shift operations! Thus, over/underflows can occur.

The left shift ``<<`` operator shifts all the beats in the first operand by the number specified in the second operand. Shifting an operand by 1 position is equivalent to multiplying it by 2, shifting 2 positions is equivalent to multiplying it by 4, and shifting 3 positions is equivalent to multiplying by 8.
The left shift `<<` operator shifts all the bits in the first operand by the number specified in the second operand. Shifting an operand by 1 position is equivalent to multiplying it by 2, shifting 2 positions is equivalent to multiplying it by 4 and shifting 3 positions is equivalent to multiplying by 8.

```solidity
uint8 public a = 100;
uint8 public b = 2;

uint8 public c = a << b; // overflow as 100 * 4 > 255
uint8 public c = a << b; // overflow as 100 * 4 > 255
```
In the above code, left shifting ``a`` which is ``100`` by 2 positions ``b`` is equivalent to multiplying 100 by 4. So the result will overflow and the value in c will be ``144`` because ``400-256`` is ``144``.

In the above code, left shifting `a` which is `100` by 2 positions `b`, is equivalent to multiplying 100 by 4. So the result will overflow and the value in c will be `144` because `400-256` is `144`.

### Use of Inline Assembly:
Inline Assembly in solidity is performed using YUL language. In YUL programming language, integer underflow & overflow is possible as compiler does not check automatically for it as YUL is a low-level language that is mostly used for making the code more optimized, which does this by omitting many opcodes.

In Solidity, inline assembly/YUL allows for low-level programming directly with EVM opcodes. This can be powerful but also risky because it lacks the built-in safety checks provided by Solidity, such as protections against overflow and underflow in arithmetic operations even if the Solidity version is >=v0.8

Consider the contract below:

```solidity
uint8 public a = 255;

function addition() public returns (uint8 result) {
assembly {
result := add(sload(a.slot), 1) // adding 1 will overflow and reset to 0
// using inline assembly
}

return result;
}
```
In the above code we are adding ``1`` into the variable with inline assembly and then returning the result. The variable result will overflow and 0 will be returned, despite this the contract will not throw an error or revert.

In the above code we are adding 1 into the variable with inline assembly and then returning the result. The variable result will overflow and 0 will be returned, despite this the contract will NOT throw an error or revert!


### Subtle Overflow with Smaller Integers (e.g., `uint128`)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda feel like this section just leads to confusion because it's really quite similar to what would happen when not dropping down to assembly

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well actually, my initial assumption was that it reverts when we try to return outputTokens as a uint128, but perhaps this isn't the case? If so this would be good to clarify

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda feel like this section just leads to confusion because it's really quite similar to what would happen when not dropping down to assembly

which section specifically are you talking about?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well actually, my initial assumption was that it reverts when we try to return outputTokens as a uint128, but perhaps this isn't the case? If so this would be good to clarify

about this @kadenzipfel, it took me a while to wrap my head around it, but here is what I found out:-


  1. Function Definition:

    function getSwapQuoteUint128(uint128 amountToSwap) external view returns (uint128 outputTokens) {
        assembly {
            outputTokens := add(amountToSwap, 1)
            if lt(outputTokens, amountToSwap) { revert(0, 0) }
        }
    }

    This function is intended to return amountToSwap + 1 and detect an overflow if outputTokens is less than amountToSwap.

  2. 256-bit Arithmetic:
    In the EVM, arithmetic operations within inline assembly (Yul) are performed using 256-bit values. When adding two 128-bit values, the result is treated as a 256-bit value.

  3. Overflow Check:
    The function includes an overflow detection mechanism:

    if lt(outputTokens, amountToSwap) { revert(0, 0) }

    This checks if outputTokens is less than amountToSwap.

  4. Overflow Scenario:
    When amountToSwap is type(uint128).max (2^128 - 1), adding 1 results in 2^128, which fits in a 256-bit register but exceeds the 128-bit range. The result is a valid 256-bit value (2^128), but it cannot be represented as a 128-bit value.

  5. Implicit Conversion:
    When the 256-bit result is assigned to a 128-bit variable (outputTokens), it truncates the higher bits, resulting in 0 (since 2^128 overflows a 128-bit integer back to 0).

  6. Assertion:
    The assertion confirms that the function indeed returns 0 when given the maximum uint128 value due to the overflow:

    assertEq(0, dexPair.getSwapQuoteUint128(type(uint128).max));

The key issue here is that the overflow detection mechanism:

if lt(outputTokens, amountToSwap) { revert(0, 0) }

fails because outputTokens and amountToSwap are treated as 256-bit values during the addition operation. When outputTokens is 2^128, it is not less than amountToSwap in 256-bit arithmetic. Therefore, the check lt(outputTokens, amountToSwap) does not trigger a revert, even though the actual result overflows when considering 128-bit constraints.

TLDR;

The inline assembly overflow check fails because the EVM uses 256-bit arithmetic, while the function is supposed to handle 128-bit values. This discrepancy allows an overflow to evade detection, causing the function to return incorrect results without reverting. Specifically, adding 1 to the maximum uint128 value (2^128 - 1) results in an overflow to 0 instead of triggering the overflow detection.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think this section is has too much overlap with the above section for it to be necessary


When using smaller integers like `uint128`, a more subtle overflow can occur because inline assembly operates with 256-bit values.

Consider the contract below which provides a method to get a swap quote by adding one to the input amount:

```solidity
interface IDexPair {
function getSwapQuoteUint128(uint128 amountToSwap) external view returns(uint128);
}

contract DexPair is IDexPair {
function getSwapQuoteUint128(uint128 amountToSwap) external view returns(uint128 outputTokens) {
assembly {
outputTokens := add(amountToSwap, 1)
if lt(outputTokens, amountToSwap) { revert(0, 0) }
}
}
}
```

- The problem is that the `add` opcode always produces a 256-bit result. For `uint128` maximum value (`type(uint128).max`), this will not overflow in the 256-bit space but will overflow when treated as `uint128`
- The overflow check inside assembly fails because it checks against 256-bit values.


### Use of unchecked code block:
Performing arithmetic operations inside the unchecked block saves a lot of gas because it omits several checks and opcodes. But, some of these opcodes are used in default arithmetic operations in 0.8 to check for underflow/overflow.
Performing arithmetic operations inside the unchecked block saves a lot of gas because it omits several checks and opcodes. But some of these opcodes are used in default arithmetic operations in 0.8 to check for underflow/overflow.

```solidity
uint8 public a = 255;

function uncheck() public{

unchecked {
a++; // overflow and reset to 0 without reverting
a++; // overflow and reset to 0 without reverting
}

}
```
The unchecked code block is only recommended if you are sure that there is no possible way for the arithmetic to overflow or underflow.


### Mitigation Strategies

1. Using `addmod`

```solidity
function getSwapQuoteUint128(uint128 amountToSwap) external view returns(uint128 outputTokens) {
assembly {
outputTokens := addmod(amountToSwap, 1, 340282366920938463463374607431768211455)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most circumstances, we'd rather have execution revert rather than actually overflowing the output. In fact, I can't really think of any circumstances where we wouldn't

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I don't think we should suggest allowing the overflow

if lt(outputTokens, amountToSwap) { revert(0, 0) }
}
}
```

The `addmod` operation confines the result within the `uint128` range.

2. Post-assembly Check

```solidity
function getSwapQuoteUint128(uint128 amountToSwap) external view returns(uint128 outputTokens) {
assembly {
outputTokens := add(amountToSwap, 1)
}
require(outputTokens >= amountToSwap, "Overflow detected!");
}
```

Perform an overflow check outside YUL using normal Solidity to ensure correct behavior for `uint128` as it will compare the 128-bit values.

3. Add a manual check

```solidity
function getSwapQuote(uint256 amountToSwap) external view returns(uint256 outputTokens) {
assembly {
outputTokens := add(amountToSwap, 1)
if lt(outputTokens, amountToSwap) { revert(0, 0) }
}
}
```

This check ensures that if the result is less than the input, it indicates an overflow.

> [!IMPORTANT]
> Be careful not to fall into the pitfall of the aforementioned `uint128`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not clear on what this note is indicating

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was supposed to indicate the pitfall a dev may fall into when implementing an overflow check in assembly using the patterns discussed.

Perhaps it wasn't clear :)



## Sources
1. https://docs.soliditylang.org/en/latest/080-breaking-changes.html
2. https://faizannehal.medium.com/how-solidity-0-8-protect-against-integer-underflow-overflow-and-how-they-can-still-happen-7be22c4ab92f