Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Add jump DDOS protection #710

Merged
merged 1 commit into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions evm_arithmetization/src/cpu/kernel/asm/core/exception.asm
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,14 @@ global invalid_jump_jumpi_destination_common:
{
// We have a jump destination on the stack. We want to `PANIC` if it is valid, and jump to
// `fault_exception` if it is not. An address is a valid jump destination if it points to a
// `JUMPDEST` instruction. In practice, since in this implementation memory addresses are
// limited to 32 bits, we check two things:
// 1. the address is no more than 32 bits long, and
// `JUMPDEST` instruction. In practice, we check two things:
// 1. the address is no greater than MAX_CODE_SIZE, and
// 2. it points to a `JUMPDEST` instruction.
// stack: jump_dest
DUP1
%shr_const(32)
PUSH @MAX_CODE_SIZE
DUP2
// stack: jump_dest, max_size, jump_dest
GT // jump_dest > max_size == !(jump_dest <= max_size)
%jumpi(fault_exception) // This keeps one copy of jump_dest on the stack, but that's fine.
// jump_dest is a valid address; check if it points to a `JUMP_DEST`.
DUP1
Expand Down
11 changes: 7 additions & 4 deletions evm_arithmetization/src/cpu/kernel/constants/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,11 +371,14 @@ const PRECOMPILES_GAS: [(&str, u16); 14] = [

const SNARKV_POINTERS: [(&str, u64); 2] = [("SNARKV_INP", 112), ("SNARKV_OUT", 100)];

pub(crate) const MAX_CODE_SIZE: u64 = if cfg!(feature = "polygon_pos") {
0x8000 // Polygon PoS value, see PIP-30.
} else {
0x6000 // default Ethereum value
};

const CODE_SIZE_LIMIT: [(&str, u64); 3] = [
#[cfg(not(feature = "polygon_pos"))]
("MAX_CODE_SIZE", 0x6000), // default Ethereum value
#[cfg(feature = "polygon_pos")]
("MAX_CODE_SIZE", 0x8000), // Polygon PoS value, see PIP-30.
("MAX_CODE_SIZE", MAX_CODE_SIZE),
("MAX_INITCODE_SIZE", 0xc000),
("INITCODE_WORD_COST", 2),
];
Expand Down
4 changes: 4 additions & 0 deletions evm_arithmetization/src/cpu/kernel/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,10 @@ impl<F: RichField> State<F> for Interpreter<F> {
self.generation_state.incr_pc(n);
}

fn is_kernel(&self) -> bool {
self.is_kernel()
}

fn get_registers(&self) -> RegistersState {
self.generation_state.get_registers()
}
Expand Down
7 changes: 7 additions & 0 deletions evm_arithmetization/src/generation/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ pub(crate) trait State<F: RichField> {
/// Returns a `State`'s stack.
fn get_stack(&self) -> Vec<U256>;

/// Indicates whether we are in kernel mode.
fn is_kernel(&self) -> bool;

/// Returns the current context.
fn get_context(&self) -> usize;

Expand Down Expand Up @@ -643,6 +646,10 @@ impl<F: RichField> State<F> for GenerationState<F> {
self.stack()
}

fn is_kernel(&self) -> bool {
self.registers.is_kernel
}

fn get_context(&self) -> usize {
self.registers.context
}
Expand Down
10 changes: 10 additions & 0 deletions evm_arithmetization/src/witness/transition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use super::util::stack_pop_with_log_and_fill;
use crate::cpu::columns::CpuColumnsView;
use crate::cpu::kernel::aggregator::KERNEL;
use crate::cpu::kernel::constants::context_metadata::ContextMetadata;
use crate::cpu::kernel::constants::MAX_CODE_SIZE;
use crate::cpu::kernel::opcodes::get_opcode;
use crate::cpu::membus::NUM_GP_CHANNELS;
use crate::cpu::stack::{
Expand Down Expand Up @@ -383,6 +384,10 @@ where
.try_into()
.map_err(|_| ProgramError::InvalidJumpDestination)?;

if !self.is_kernel() && dst > MAX_CODE_SIZE as u32 {
return Err(ProgramError::InvalidJumpDestination);
}

if !self.generate_jumpdest_analysis(dst as usize) {
row.mem_channels[1].value[0] = F::ONE;

Expand Down Expand Up @@ -450,6 +455,11 @@ where
let dst: u32 = dst
.try_into()
.map_err(|_| ProgramError::InvalidJumpiDestination)?;

if !self.is_kernel() && dst > MAX_CODE_SIZE as u32 {
return Err(ProgramError::InvalidJumpiDestination);
}

if !self.generate_jumpdest_analysis(dst as usize) {
row.general.jumps_mut().should_jump = F::ONE;
let cond_sum_u64 = cond
Expand Down
6,033 changes: 6,033 additions & 0 deletions trace_decoder/tests/cases/b978_dev.json

Large diffs are not rendered by default.

Loading
Loading