TOB-FUEL-1: Incorrect block hash returned for historical blocks

[xgreenx]

Description

Missing validation in the blockhashAtCommit function results in an incorrect blockHash being returned for any commitHeight that is more than NUM_COMMIT_SLOTS behind the latest commit height. This might cause problems in frontend applications and other smart contract integrations that rely on the correct return value of this function.

Figure 1.1: The blockHashAtCommit function in fuel-v2-contracts/contracts/fuelchain/FuelChainState.sol:

116    function blockHashAtCommit(uint256 commitHeight) external view returns
(bytes32) {
117       Commit storage commitSlot = _commitSlots[commitHeight % NUM_COMMIT_SLOTS];
118       return commitSlot.blockHash;
119    }

The FuelChainState contract stores the latest NUM_COMMIT_SLOTS (figure 1.2) commits in an array (figure 1.3). Whenever a new commit is added a modulo operation is used to choose the correct index in this array (figure 1.4) at which to insert the new commit.

Figure 1.2: The NUM_COMMIT_SLOTS constant variable in fuel-v2-contracts/contracts/fuelchain/FuelChainState.sol

27    uint256 public constant NUM_COMMIT_SLOTS = 240; //30 days worth of commits

Figure 1.3: The _commitSlots array variable in fuel-v2-contracts/contracts/fuelchain/FuelChainState.sol

47    Commit[NUM_COMMIT_SLOTS] private _commitSlots;

Figure 1.4: The commit function in fuel-v2-contracts/contracts/fuelchain/FuelChainState.sol

88    function commit(bytes32 blockHash, uint256 commitHeight) external
whenNotPaused onlyRole(COMMITTER_ROLE) {
89       uint256 slot = commitHeight % NUM_COMMIT_SLOTS;
90       Commit storage commitSlot = _commitSlots[slot];
91       commitSlot.blockHash = blockHash;
92       commitSlot.timestamp = uint32(block.timestamp);
93
94       emit CommitSubmitted(commitHeight, blockHash);
95    }

If the blockHashAtCommit function is called with a commitHeight that is sufficiently far in the past, then the blockHash of another (a newer) commit will be returned. What instead should happen is that the function reverts if it cannot return the blockHash of the requested commit height.

Exploit Scenario

Alice, a developer of a frontend application that interacts with the Fuel smart contracts, develops a UI component to retrieve the block hash of at any given commit height. The UI component seems to work as it returns a block hash for every commit height that is requested. However, it shows an incorrect block hash for all sufficiently old commit heights.

Recommendations

Short term, update the blockHashAtCommit function to revert if it cannot return the blockHash of the requested commitHeight. For example, by adding a field height to the Commit struct and checking that commitHeight == commit.height.
Long term, take into consideration other applications that interact with the Fuel smart contracts and ensure that all view functions return correct results at all times.

[DefiCake]

This is a non issue and was addressed during previous calls. It basically tracks a difficulty offchain / for the frontend. Addressing it would imply increasing the gas costs for the block committer, which we want to keep to a minimum.

I suggest that we acknowledge it to ToB as a won't fix.