Bug Report: GameBitboard Missing Empty-Cell Check

[Preshy]

Summary

The moveField function in GameBitboard.fe does not verify that the empty-cell search was successful. If the board has no zero cell, empty_idx remains at 16, causing the contract to read from uninitialized storage and allow illegal moves.

Affected Contract

contracts/ingots/games/src/game_bitboard.fe — the MoveField handler (lines 92-118).

Root Cause

// In MoveField handler:
let mut empty_idx: u256 = 16
let mut j: u256 = 0
while j < 16 {
    if get_cell(board: store.board, index: j) == 0 {
        empty_idx = j
    }
    j += 1
}

// Missing: no check that empty_idx was actually found!
let encoded = store.moves.get(key: empty_idx)  // reads key 16 → returns 0
if !is_movable_to(encoded_moves: encoded, target: index) {
    revert(Error::NotMovable)
}

When no cell contains 0, empty_idx stays at 16. The subsequent store.moves.get(16) reads from a storage slot that was never initialized, returning 0. is_movable_to(0, 0) evaluates to true, allowing the move to proceed with invalid board state.

The companion game.fe (StorageMap variant) IS protected because ADJACENCY[16] would cause an out-of-bounds array access (which the compiler correctly bounds-checks), but game_bitboard.fe stores adjacency in a StorageMap which has no bounds checking.

Proof of Concept

Run: forge test --match-contract BugProofTest -vvv

function test_move_field_succeeds_on_zero_free_board() public {
    // Board: no zero cell (cell 15 = 1 instead of 0)
    uint256 board = 0x1EFDCBA987654321;
    address gameAddr = _deployBoardedGame(board);
    IGame1D game = IGame1D(gameAddr);

    assertFalse(ISolvable(gameAddr).isSolved());
    game.moveField(0);  // This SHOULD revert but SUCCEEDS
    assertEq(game.getBoard(0), 0);  // Cell 0 is now 0
}

Impact

A zero-free board becomes exploitable: calling moveField(0) injects an empty cell, potentially making an unsolvable board solvable. The deployed mainnet contracts are NOT vulnerable because they use the standard UNSOLVABLE_BOARD which contains a zero at position 15.

However, any future deployment with a custom board configuration (including a board where cell 15 is non-zero) would be vulnerable. The bug also represents a logic flaw that could combine with other issues.

Recommended Fix

Add a check after the empty-cell search:

let encoded = store.moves.get(key: empty_idx)
// Add these lines:
if empty_idx == 16 {
    revert(Error::InvalidIndex)  // or a new error: NoEmptyCell
}
if !is_movable_to(encoded_moves: encoded, target: index) {
    revert(Error::NotMovable)
}

Or alternatively, initialize empty_idx to an out-of-bounds sentinel and check:

let mut empty_idx: u256 = 16  // sentinel value
// ... search loop ...
assert(empty_idx != 16)  // Fe's assert reverts on false

Affected Variants

Variant	Vulnerable?	Reason
Game	No	Uses ADJACENCY[empty_idx] which bounds-checks
Game2D	No	Uses array indexing with compiler bounds checks
GameEnum	No	Uses array indexing
GameBitboard	Yes	Uses StorageMap (no bounds check)
GameTrait	Likely safe	Array-based
GameNested	Likely safe	Struct-based
GameMonadic	Likely safe	Single u256

Verification

cd bountiful
make build
forge test --match-contract BugProofTest -vvv

All tests pass, confirming the vulnerability.