# RACE #10 - Test Cases

{% embed url="<https://ventral.digital/posts/2022/10/3/secureum-bootcamp-epoch-october-race-10>" %}
RACE #10
{% endembed %}

<figure><img src="/files/08FPI58yzdDbTKjDD5es" alt=""><figcaption><p>RACE #10 result</p></figcaption></figure>

{% hint style="info" %}
*Note: All 8 questions in this RACE are based on the below contract. This is the same contract you will see for all the 8 questions in this RACE. The question is below the shown contract.*
{% endhint %}

```solidity
// SPDX-License-Identifier: MIT
pragma solidity 0.8.17;
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";

contract TestContract is Ownable {

    function Test1(uint n) external pure returns(uint) {
        return n + abi.decode(msg.data[msg.data.length-64:], (uint));
    }

    function Test2(uint n) public view returns(uint) {
        bytes memory fcall = abi.encodeCall(TestContract.Test1,(n));
        bytes memory xtr = abi.encodePacked(uint(4),uint(5));
        bytes memory all = bytes.concat(fcall,xtr);
        (bool success, bytes memory data) = address(this).staticcall(all);
        return abi.decode(data,(uint));
    }

    type Nonce is uint256;
    struct Book { Nonce nonce;}

    function NextBookNonce(Book memory x) public pure {
       x.nonce = Nonce.wrap(Nonce.unwrap(x.nonce) + 3);
    }

    function Test3(uint n) public pure returns (uint) {
      Book memory bookIndex;
      bookIndex.nonce = Nonce.wrap(7);
      for (uint i=0;i<n;i++) {
         NextBookNonce(bookIndex);
      }
      return Nonce.unwrap(bookIndex.nonce);
    }

    error ZeroAddress();
    error ZeroAmount();
    uint constant ZeroAddressFlag = 1;
    uint constant ZeroAmountFlag = 2;

    function process(address[] memory a, uint[] memory amount) public pure returns (uint){
        uint error;
        uint total;
        for (uint i=0;i<a.length;i++) {
            if (a[i] == address(0)) error |= ZeroAddressFlag;
            if (amount[i] == 0) error |= ZeroAmountFlag;
            total += amount[i];
        }
        if (error == ZeroAddressFlag) revert ZeroAddress();
        if (error == ZeroAmountFlag)  revert ZeroAmount();
        return total;
    }

    function Test4(uint n) public pure returns (uint) {
        address[] memory a = new address[](n+1);
        for (uint i=0;i<=n;i++) {
            a[i] = address(uint160(i));
        }
        uint[] memory amount = new uint[](n+1);
        for (uint i=0;i<=n;i++) {
            amount[i] = i;
        }
        return process(a,amount);
    }

    uint public totalMinted;
    uint constant maxMinted = 100;
    event minted(uint totalMinted,uint currentMint);

    modifier checkInvariants() {
        require(!paused, "Paused");
        _;
        invariantCheck();
        require(!paused, "Paused");
    }

    function invariantCheck() public {
        if (totalMinted > maxMinted) // this may never happen
            pause();
    }

    bool public paused;
    function pause() public {
        paused = true;
    }
    function unpause() public onlyOwner {
        paused = false;
    }

    function Test5( uint n) public checkInvariants(){
        totalMinted += n;
        emit minted(n,totalMinted);
    }
}
```

## Question 1 :white\_check\_mark:

Which statements are true in *Test1()*?

* [x] &#x20;A. The function does not use all supplied extra data -> <mark style="color:red;">**Not sure what this means**</mark>
* [x] &#x20;B. The function can revert due to an underflow -> `msg.data.length-64`
* [x] &#x20;C. The function can revert due to an overflow -> `n + ...`
* [ ] &#x20;D. The function accesses memory which it should not

**Comment:**

Answer A seems a bit confusing when looking at *Test1()* alone, but seeing the *xtr* variable of *Test2()* brings some clarity: The *Test1()* function signature expects one uint to be passed, but then within the function body it loads 64 bytes directly from calldata. *Test2()* then shows how the function is intended to be called by concatenating extra data to the ABI encoded calldata. It adds two more uint types which together are 64 bytes of extra data. But then in the *abi.decode* only the first uint from extra data is actually decoded and used.

Both B and C are true since a Solidity version (^0.8.0) is used, that automatically checks for integer over/underflows and reverts when these happen. In this specific case, an overflow could happen when parameter *n* or the number supplied from extra data are large enough to wrap. The underflow can happen when the overall supplied calldata is smaller than 64 bytes, making the subtraction within the slicing parameters fail.

You could argue that accessing *msg.data* directly should be avoided when possible. But this doesn't access memory but read-only calldata. Therefore no memory is accessed that should not be.

## Question 2 :white\_check\_mark:

Which statements are true in *Test2()*?

* [x] &#x20;A. Result of *encodePacked* is deterministic -> Of course `abi.encodePacked(uint(4),uint(5))` is deterministic
* [ ] &#x20;B. *abi.decode* always succeeds -> Only succeeds when `fcall` returns legit data, otherwise the staticcall will fail
* [x] &#x20;C. It calls function *Test1()* without any problem -> `abi.encodeCall(TestContract.Test1,(n))` is good
* [ ] &#x20;D. It should use *uint256* instead of *uint ->* `uint` is just an alias for `uint256`

**Comment:**

Deterministic means that you should always get the same predictable output for a given input. As such, *encodePacked* always encodes passed data the same way.

Test2's *abi.decode* will only succeed if no error happens in *Test1()*. If *Test1()* reverts the returned data would not contain a decodable uint but error data. <mark style="color:red;">**One way to cause this to happen would be supplying a number**</mark><mark style="color:red;">**&#x20;**</mark>*<mark style="color:red;">**n**</mark>*<mark style="color:red;">**&#x20;**</mark><mark style="color:red;">**that causes an overflow. The best practice is to check the**</mark><mark style="color:red;">**&#x20;**</mark>*<mark style="color:red;">**success**</mark>*<mark style="color:red;">**&#x20;**</mark><mark style="color:red;">**boolean before attempting to decode the returned data.**</mark>

Answer D leaves some room for interpretation. *uint* is an alias of *uint256* and there should not be an issue using it here. But it's a common best practice to avoid the shorter alias and instead use the longer-named version of the type. While this is generally considered to improve readability, I'd argue that consistency (always using the same type) is more important.

## Question 3 :white\_check\_mark:

Which statements are true in *NextBookNonce()*?

* [ ] &#x20;A. *wrap* and *unwrap* cost additional gas -> No effect from EVM's perspective
* [x] &#x20;B. It is safe to use *unchecked ->* `x.nonce + 3` is a small increment, exploiting this is expensive
* [x] &#x20;C. There is something suspicious about the increment value -> Nonce usually increments by 1
* [ ] &#x20;D. It could have used *x.nonce++*

**Comment:**

<mark style="color:red;">**The calls to**</mark><mark style="color:red;">**&#x20;**</mark>*<mark style="color:red;">**wrap**</mark>*<mark style="color:red;">**&#x20;**</mark><mark style="color:red;">**and**</mark><mark style="color:red;">**&#x20;**</mark>*<mark style="color:red;">**unwrap**</mark>*<mark style="color:red;">**&#x20;**</mark><mark style="color:red;">**are basically telling Solidity whether it should treat a certain variable as being of a custom type (**</mark>*<mark style="color:red;">**Nonce**</mark>*<mark style="color:red;">**) or of its native type (**</mark>*<mark style="color:red;">**uint256**</mark>*<mark style="color:red;">**). This switch is basically just syntactic sugar for handling types within Solidity, the EVM will know nothing of these type switches and no additional gas will be used by doing so.**</mark>

Using an *unchecked* block in this function would omit Solidity's over/underflow handling. Especially in the context of a Nonce (Number used only once), you don't want integer values to wrap and overflow to values that were once used before. But usually, a nonce is only increased by such a small value that exploiting this would be very expensive. In this specific case, the function is pure and the nonce is not stored, so whether it's safe to use unchecked block will depend on the function being used correctly.

Answer C sounds rather ominous but it's simply pointing out that Nonces are commonly increased by one and not by such a weird number as 3.

Arithmetic operations cannot be executed on custom types without unwrapping the number first.

## Question 4 :white\_check\_mark:

Which statements are true in *Test3()*?

* [x] &#x20;A. *bookIndex.nonce* is incremented in the loop -> `NextBookNonce()` is called in the for loop
* [ ] &#x20;B. *bookIndex.nonce* cannot be incremented because *NextBookNonce* is pure -> `bookIndex` is stored in memory, so pure function can modify it
* [x] &#x20;C. *i++* can be made *unchecked ->* `i++` won't overflow because `NextBookNonce()` increments by 3 on each step and `i++` increments by 1 on each step. If overflow occurs, `NextBookNonce()` will overflow first and that will be protected by Solidity compiler.
* [ ] &#x20;D. *memory* can be changed to *storage* without any other changes

**Comment:**

Both A and B should be clear from reading the code.

The increment of *i* within the loop can indeed be made *unchecked* since it won't be able to overflow no matter what is supplied as *n*.

The *memory* location can't simply be changed to *storage* without various further changes such as assigning it to a specific storage slot before being able to make use of it.

## Question 5 :white\_check\_mark:

Which statements are true In *Test4()*?

* [ ] &#x20;A. The function always reverts with *ZeroAddress()*
* [ ] &#x20;B. The function always reverts with *ZeroAmount()*
* [x] &#x20;C. The function never reverts with *ZeroAddress()*
* [x] &#x20;D. The function never reverts with *ZeroAmount()*

**Comment:**

The first array elements of both *a* and *amounts* will always be zero-like. Both *1* for *ZeroAddress* and *2* for *ZeroAmount* will be OR-combined resulting in 3. Once this value is set as an error, further iterations will not influence it. After the loop has finished, this error value is not checked for and instead, the function returns the *total* without reverting.

**My comment:**

Some OR boolean algebra:

```solidity
function process(address[] memory a, uint[] memory amount) public pure returns (uint){
    uint error;
    uint total;
    for (uint i=0;i<a.length;i++) {
        // @audit a[0] and amount[0] are zero-like
        // error = 1 | 2 = 0b01 | 0b10 = 0b11 = 3
        // 3 | 1 = 0b11 | 0b01 = 0b11 = 3
        // 3 | 2 = 0b11 | 0b10 = 0b11 = 3
        if (a[i] == address(0)) error |= ZeroAddressFlag;
        if (amount[i] == 0) error |= ZeroAmountFlag;
        total += amount[i];
    }
    // @audit `error` will be set to 3 for all i >= 1
    // That means the following two if conditions will never be satisfied
    if (error == ZeroAddressFlag) revert ZeroAddress();
    if (error == ZeroAmountFlag)  revert ZeroAmount();
    return total;
}
```

## Question 6 :white\_check\_mark:

Which statements are true in *Test5()*?

* [ ] &#x20;A. modifier *checkInvariants* will pause the contract if too much is minted
* [x] &#x20;B. modifier *checkInvariants* will never pause the contract
* [ ] &#x20;C. modifier *checkInvariants* will always pause the contract
* [x] &#x20;D. There are more efficient ways to handle the *require*

**Comment:**

While the *checkInvariants* modifier does intend to pause the contract if too much is minted, it'll be unable to ever do so since this will be reverted by the second *require* call.

<mark style="color:red;">**A single call to**</mark><mark style="color:red;">**&#x20;**</mark>*<mark style="color:red;">**require**</mark>*<mark style="color:red;">**&#x20;**</mark><mark style="color:red;">**would fix this issue and also be more efficient.**</mark>

**My comment:**

The modifier `checkInvariants()` has a logic issue:

```solidity
modifier checkInvariants() {
    require(!paused, "Paused");
    _;
    invariantCheck();
    // @audit Even `pause()` is called in `invariantCheck()`
    // The whole tx will be reverted because of the following require statement
    require(!paused, "Paused");
}
```

## Question 7 :white\_check\_mark:

Which statements are true about the *owner*?

* [x] &#x20;A. The *owner* is initialized -> `owner` is inherited from Ownable
* [ ] &#x20;B. The *owner* is not initialized
* [ ] &#x20;C. The *owner* cannot be changed
* [x] &#x20;D. The *owner* can be changed -> `transferOwnership()` is inherited from Ownable

**Comment:**

Although not visible here, the *owner* is indeed initialized by the constructor inherited from *Ownable*, which also comes with functions allowing to change the *owner* at a later point.

## Question 8 :white\_check\_mark:

Which statements are true in *Test5()* and related functions?

* [x] &#x20;A. *pause* is unsafe -> `onlyOwner` modifier is missing
* [ ] &#x20;B. *unpause* is unsafe
* [ ] &#x20;C. The *emit* is done right
* [x] &#x20;D. The *emit* is done wrong -> Should be `emit minted(totalMinted, n)`

**Comment:**

The *pause* function is missing the *onlyOwner* modifier allowing anyone to arbitrarily pause the contract.

The *minted* event's parameters appear to be in the wrong order.


---

# Agent Instructions: Querying This Documentation

If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter:

```
GET https://ret2basic.gitbook.io/ctfnote/web3-security-research/secureum/epoch-infinity/race-10-test-cases.md?ask=<question>
```

The question should be specific, self-contained, and written in natural language.
The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.