Skip to content

Commit

Permalink
[KGA-51] fix: underconstrained hint in initialize_jumpdests (#1626)
Browse files Browse the repository at this point in the history
Fixes an underconstrained hint in initialize_jumpdests. Avoids a
malicious prover to continue iterating over the bytecode limit and
consuming extra resources, but has no impact over code execution

code-423n4/2024-09-kakarot-findings#26

---------

Co-authored-by: Clément Walter <[email protected]>
  • Loading branch information
enitrat and ClementWalter authored Nov 20, 2024
1 parent 9b4a593 commit 204b3d7
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 1 deletion.
13 changes: 13 additions & 0 deletions cairo_zero/tests/src/utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,19 @@ async def test_should_return_same_as_execution_specs(self, cairo_run):
output = cairo_run("test__initialize_jumpdests", bytecode=bytecode)
assert set(output) == get_valid_jump_destinations(bytecode)

async def test_should_err_on_malicious_prover(self, cairo_program, cairo_run):
with (
patch_hint(
cairo_program,
"memory[ap] = 0 if 0 <= (ids.a % PRIME) < range_check_builtin.bound else 1",
"memory[ap] = 1",
"initialize_jumpdests",
),
cairo_error(message="Reading out of bounds bytecode"),
):
bytecode = (await get_contract("PlainOpcodes", "Counter")).bytecode_runtime
cairo_run("test__initialize_jumpdests", bytecode=bytecode)


class TestLoadPackedBytes:
def test_should_load_packed_bytes(self, cairo_run):
Expand Down
5 changes: 5 additions & 0 deletions cairo_zero/utils/utils.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,11 @@ namespace Helpers {
let valid_jumpdests = cast([ap - 2], DictAccess*);
let i = [ap - 1];

with_attr error_message("Reading out of bounds bytecode") {
assert [range_check_ptr] = bytecode_len - 1 - i;
}
let range_check_ptr = range_check_ptr + 1;

tempvar opcode = [bytecode + i];
let is_opcode_ge_0x5f = Helpers.is_le_unchecked(0x5f, opcode);
let is_opcode_le_0x7f = Helpers.is_le_unchecked(opcode, 0x7f);
Expand Down
19 changes: 18 additions & 1 deletion tests/utils/hints.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from collections import defaultdict
from contextlib import contextmanager
from typing import Optional
from unittest.mock import patch

from starkware.cairo.common.dict import DictTracker
Expand Down Expand Up @@ -33,12 +34,28 @@ def new_default_dict(


@contextmanager
def patch_hint(program, hint, new_hint):
def patch_hint(program, hint, new_hint, scope: Optional[str] = None):
"""
Patch a Cairo hint in a program with a new hint.
Args:
program: The Cairo program containing the hints
hint: The original hint code to replace
new_hint: The new hint code to use instead
scope: Optional scope name to restrict which hints are patched. If provided,
only hints in scope containing this string will be patched.
Example:
with patch_hint(program, "old_hint", "new_hint", "initialize_jumpdests"):
# Code that runs with the patched hint
"""
patched_hints = {
k: [
(
hint_
if hint_.code != hint
or (scope is not None and scope not in str(hint_.accessible_scopes[-1]))
else CairoHint(
accessible_scopes=hint_.accessible_scopes,
flow_tracking_data=hint_.flow_tracking_data,
Expand Down

0 comments on commit 204b3d7

Please sign in to comment.