Skip to content

Fix critical security vulnerabilities and EVM compliance issues#67

Merged
tcrypt25519 merged 7 commits into
mainfrom
copilot/add-stack-bounds-checking
Feb 14, 2026
Merged

Fix critical security vulnerabilities and EVM compliance issues#67
tcrypt25519 merged 7 commits into
mainfrom
copilot/add-stack-bounds-checking

Conversation

Copilot AI commented Feb 14, 2026

Copy link
Copy Markdown

Addresses stack underflow memory corruption, ADDMOD/MULMOD arithmetic overflow, and use-after-free in memory operations. BYTE instruction verified correct, no changes needed.

Stack Underflow Protection

Stack pop operations now check for underflow before decrementing stack_ptr. Returns null pointer on underflow, detected by caller which returns StackUnderflow error instead of wrapping to u32::MAX and accessing arbitrary memory.

// In build_stack_pop - check before decrement
let is_underflow = self.builder.build_int_compare(
    inkwell::IntPredicate::EQ,
    stack_ptr,
    self.types.i32.const_zero(),
    "is_underflow",
)?;
// Branch to return null if underflow detected

ADDMOD/MULMOD 512-bit Intermediate Arithmetic

EVM spec requires unlimited precision for (a + b) % N and (a * b) % N to prevent overflow before modulo. Implemented runtime builtins using bnum's U512 for intermediate calculations:

// Convert to 512-bit, perform operation, then mod and downcast
let a_u512 = u256_to_u512(a_u256);
let b_u512 = u256_to_u512(b_u256);
let sum = a_u512 + b_u512;  // No overflow possible
let result = (sum % n_u512).as_u256();

Handles division-by-zero per spec (returns 0). Symbol mappings added to execution engine.

Memory UAF Prevention

mem_load previously returned pointer into memory buffer. When mem_expand reallocates, those pointers become dangling. Changed to return i256 value directly:

// Old: returned pointer that could dangle
let byte_ptr = /* calculate pointer into memory buffer */;
self.builder.build_return(Some(&byte_ptr))

// New: load and return value immediately
let value = self.builder.build_load(self.types.i256, byte_ptr, "mem.value")?;
self.builder.build_return(Some(&value))

BYTE Instruction

Verified implementation correct. The 31 - index formula properly converts big-endian EVM indices to little-endian internal representation per existing tests and spec.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@semanticdiff-com

semanticdiff-com Bot commented Feb 14, 2026

Copy link
Copy Markdown

Review changes with  SemanticDiff

Changed Files
File Status
  crates/jet/src/builder/ops.rs  22% smaller
  crates/jet_runtime/src/runtime_builder.rs  6% smaller
  crates/jet/src/builder/env.rs  0% smaller
  crates/jet/src/engine/mod.rs  0% smaller
  crates/jet_runtime/src/builtins.rs  0% smaller
  crates/jet_runtime/src/exec.rs  0% smaller
  crates/jet_runtime/src/symbols.rs  0% smaller

Copilot AI changed the title [WIP] Add stack bounds checking to prevent memory corruption Fix critical security vulnerabilities and EVM compliance issues Feb 14, 2026
Copilot AI requested a review from tcrypt25519 February 14, 2026 05:09
Copilot AI and others added 7 commits February 14, 2026 16:10
Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
@tcrypt25519
tcrypt25519 force-pushed the copilot/add-stack-bounds-checking branch from cd7184c to aba4bd2 Compare February 14, 2026 22:16
@tcrypt25519
tcrypt25519 marked this pull request as ready for review February 14, 2026 22:16
Copilot AI review requested due to automatic review settings February 14, 2026 22:17
@tcrypt25519
tcrypt25519 merged commit d9de4ce into main Feb 14, 2026
5 checks passed
@tcrypt25519
tcrypt25519 deleted the copilot/add-stack-bounds-checking branch February 14, 2026 22:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request addresses three critical security vulnerabilities in the EVM implementation: stack underflow memory corruption, ADDMOD/MULMOD arithmetic overflow, and use-after-free in memory operations.

Changes:

  • Stack underflow protection: Adds bounds checking in stack_pop to return null pointer on underflow, with caller detecting and returning StackUnderflow error
  • ADDMOD/MULMOD 512-bit precision: Implements runtime builtins using bnum's U512 for intermediate arithmetic to prevent overflow before modulo, as required by EVM spec
  • Memory UAF fix: Changes mem_load to return i256 value directly instead of pointer, preventing dangling pointer issues when memory is reallocated

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/jet_runtime/src/symbols.rs Adds symbol constants FN_ADDMOD and FN_MULMOD for new builtin functions
crates/jet_runtime/src/runtime_builder.rs Adds LLVM function declarations for addmod/mulmod, implements stack underflow check in build_stack_pop, changes mem_load return type from ptr to i256
crates/jet_runtime/src/exec.rs Adds StackUnderflow = -2 return code enum variant
crates/jet_runtime/src/builtins.rs Implements jet_ops_addmod and jet_ops_mulmod with 512-bit intermediate arithmetic using bnum, includes helper functions for U256/U512 conversions
crates/jet/src/engine/mod.rs Registers addmod and mulmod builtins with execution engine symbol map
crates/jet/src/builder/ops.rs Updates addmod/mulmod to call new builtins, adds null pointer check in call_stack_pop for underflow detection, updates mload to handle i256 return value from mem_load
crates/jet/src/builder/env.rs Adds addmod and mulmod function value fields to Symbols struct with accessor methods

Comment on lines +916 to +939
// Check if stack underflow occurred (null pointer returned)
let is_null = bctx.builder.build_is_null(ptr, "is_stack_underflow")?;

// Create basic blocks for handling null/non-null cases
let underflow_block = bctx
.env
.context()
.append_basic_block(bctx.func, "stack_underflow");
let valid_block = bctx
.env
.context()
.append_basic_block(bctx.func, "stack_valid");

bctx.builder
.build_conditional_branch(is_null, underflow_block, valid_block)?;

// In underflow block, return with StackUnderflow error
bctx.builder.position_at_end(underflow_block);
build_return(bctx, ReturnCode::StackUnderflow)?;

// Continue in valid block
bctx.builder.position_at_end(valid_block);

Ok(ptr)

Copilot AI Feb 14, 2026

Copy link

Choose a reason for hiding this comment

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

The stack underflow detection logic is not covered by tests. Consider adding a test case that attempts to pop from an empty stack to verify that the StackUnderflow error is properly returned. This would ensure the security-critical underflow protection works correctly.

Copilot uses AI. Check for mistakes.
tcrypt25519 added a commit that referenced this pull request Feb 16, 2026
* Initial plan

* Add stack bounds checking to prevent memory corruption

Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>

* Fix Use-After-Free vulnerability in memory pointer handling

Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>

* Fix ADDMOD and MULMOD to use 512-bit precision for EVM compliance

Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>

* Add symbol mappings for ADDMOD and MULMOD builtins

Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>

* Refactor ADDMOD/MULMOD helpers to reduce code duplication

Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>

* chore: CI change 🤖

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Co-authored-by: tcrypt25519 <mail@tcry.pt>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants