Skip to content

Commit

Permalink
Check for overflows in relocatable operations (#859)
Browse files Browse the repository at this point in the history
* Catch possible overflows in Relocatable::add

* Move sub implementations to trait impl

* Swap sub_usize for - operator

* Vheck possible overflows in Add<i32>

* Fix should_panic test

* remove referenced add

* Replace Relocatable methods for trait implementations

* Catch overflows in mayberelocatable operations

* Fix keccak

* Clippy
  • Loading branch information
fmoletta authored Mar 1, 2023
1 parent 4beb4c8 commit 1b8f29e
Show file tree
Hide file tree
Showing 32 changed files with 222 additions and 237 deletions.
13 changes: 6 additions & 7 deletions src/hint_processor/builtin_hint_processor/blake2s_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,10 @@ output_ptr should point to the middle of an instance, right after initial_state,
which should all have a value at this point, and right before the output portion which will be
written by this function.*/
fn compute_blake2s_func(vm: &mut VirtualMachine, output_rel: Relocatable) -> Result<(), HintError> {
let h = get_fixed_size_u32_array::<8>(&vm.get_integer_range(output_rel.sub_usize(26)?, 8)?)?;
let message =
get_fixed_size_u32_array::<16>(&vm.get_integer_range(output_rel.sub_usize(18)?, 16)?)?;
let t = felt_to_u32(vm.get_integer(output_rel.sub_usize(2)?)?.as_ref())?;
let f = felt_to_u32(vm.get_integer(output_rel.sub_usize(1)?)?.as_ref())?;
let h = get_fixed_size_u32_array::<8>(&vm.get_integer_range((output_rel - 26)?, 8)?)?;
let message = get_fixed_size_u32_array::<16>(&vm.get_integer_range((output_rel - 18)?, 16)?)?;
let t = felt_to_u32(vm.get_integer((output_rel - 2)?)?.as_ref())?;
let f = felt_to_u32(vm.get_integer((output_rel - 1)?)?.as_ref())?;
let new_state =
get_maybe_relocatable_array_from_u32(&blake2s_compress(&h, &message, t, 0, f, 0));
let output_ptr = MaybeRelocatable::RelocatableValue(output_rel);
Expand Down Expand Up @@ -158,7 +157,7 @@ pub fn blake2s_add_uint256(
//Insert second batch of data
let data = get_maybe_relocatable_array_from_felt(&inner_data);
vm.load_data(
&MaybeRelocatable::RelocatableValue(data_ptr).add_usize(4),
&MaybeRelocatable::RelocatableValue(data_ptr).add_usize(4)?,
&data,
)
.map_err(HintError::Memory)?;
Expand Down Expand Up @@ -205,7 +204,7 @@ pub fn blake2s_add_uint256_bigend(
//Insert second batch of data
let data = get_maybe_relocatable_array_from_felt(&inner_data);
vm.load_data(
&MaybeRelocatable::RelocatableValue(data_ptr).add_usize(4),
&MaybeRelocatable::RelocatableValue(data_ptr).add_usize(4)?,
&data,
)
.map_err(HintError::Memory)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
};
use felt::Felt;
use num_traits::{ToPrimitive, Zero};
use std::{borrow::Cow, collections::HashMap, ops::Add};
use std::{borrow::Cow, collections::HashMap};

// Constants in package "starkware.cairo.common.cairo_keccak.keccak".
const BYTES_IN_WORD: &str = "starkware.cairo.common.cairo_keccak.keccak.BYTES_IN_WORD";
Expand Down Expand Up @@ -53,7 +53,7 @@ pub fn keccak_write_args(
.map_err(HintError::Memory)?;

let high_args: Vec<_> = high_args.into_iter().map(MaybeRelocatable::from).collect();
vm.write_arg(inputs_ptr.add(2_i32), &high_args)
vm.write_arg((inputs_ptr + 2_i32)?, &high_args)
.map_err(HintError::Memory)?;

Ok(())
Expand Down Expand Up @@ -145,7 +145,7 @@ pub fn block_permutation(
let keccak_state_size_felts = keccak_state_size_felts.to_usize().unwrap();
let values = vm
.get_range(
&MaybeRelocatable::RelocatableValue(keccak_ptr.sub_usize(keccak_state_size_felts)?),
&MaybeRelocatable::RelocatableValue((keccak_ptr - keccak_state_size_felts)?),
keccak_state_size_felts,
)
.map_err(HintError::Memory)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ pub fn dict_write(
let tracker = dict.get_tracker_mut(dict_ptr)?;
//dict_ptr is a pointer to a struct, with the ordered fields (key, prev_value, new_value),
//dict_ptr.prev_value will be equal to dict_ptr + 1
let dict_ptr_prev_value = dict_ptr + 1_i32;
let dict_ptr_prev_value = (dict_ptr + 1_i32)?;
//Tracker set to track next dictionary entry
tracker.current_ptr.offset += DICT_ACCESS_SIZE;
//Get previous value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub fn find_element(
if let Some(find_element_index_value) = find_element_index {
let find_element_index_usize = felt_to_usize(&find_element_index_value)?;
let found_key = vm
.get_integer(array_start + (elm_size * find_element_index_usize))
.get_integer((array_start + (elm_size * find_element_index_usize))?)
.map_err(|_| HintError::KeyNotFound)?;

if found_key.as_ref() != key.as_ref() {
Expand Down Expand Up @@ -68,7 +68,7 @@ pub fn find_element(

for i in 0..n_elms_iter {
let iter_key = vm
.get_integer(array_start + (elm_size * i as usize))
.get_integer((array_start + (elm_size * i as usize))?)
.map_err(|_| HintError::KeyNotFound)?;

if iter_key.as_ref() == key.as_ref() {
Expand Down
2 changes: 1 addition & 1 deletion src/hint_processor/builtin_hint_processor/keccak_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ pub fn unsafe_keccak_finalize(
offset: keccak_state_ptr.offset + 1,
})?;

let n_elems = end_ptr.sub(&start_ptr)?;
let n_elems = (end_ptr - start_ptr)?;

let mut keccak_input = Vec::new();
let range = vm.get_integer_range(start_ptr, n_elems)?;
Expand Down
6 changes: 3 additions & 3 deletions src/hint_processor/builtin_hint_processor/math_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,10 @@ pub fn assert_le_felt(
let (q_0, r_0) = (lengths_and_indices[0].0).div_mod_floor(prime_over_3_high);
let (q_1, r_1) = (lengths_and_indices[1].0).div_mod_floor(prime_over_2_high);

vm.insert_value(range_check_ptr + 1_i32, q_0)?;
vm.insert_value((range_check_ptr + 1_i32)?, q_0)?;
vm.insert_value(range_check_ptr, r_0)?;
vm.insert_value(range_check_ptr + 3_i32, q_1)?;
vm.insert_value(range_check_ptr + 2_i32, r_1)?;
vm.insert_value((range_check_ptr + 3_i32)?, q_1)?;
vm.insert_value((range_check_ptr + 2_i32)?, r_1)?;
Ok(())
}

Expand Down
4 changes: 3 additions & 1 deletion src/hint_processor/builtin_hint_processor/pow_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ pub fn pow(
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let prev_locs_exp = vm
.get_integer(get_relocatable_from_var_name("prev_locs", vm, ids_data, ap_tracking)? + 4_i32)
.get_integer(
(get_relocatable_from_var_name("prev_locs", vm, ids_data, ap_tracking)? + 4_i32)?,
)
.map_err(|_| {
HintError::IdentifierHasNoMember("prev_locs".to_string(), "exp".to_string())
})?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ impl BigInt3<'_> {
d0: vm.get_integer(addr).map_err(|_| {
HintError::IdentifierHasNoMember(name.to_string(), "d0".to_string())
})?,
d1: vm.get_integer(addr + 1).map_err(|_| {
d1: vm.get_integer((addr + 1)?).map_err(|_| {
HintError::IdentifierHasNoMember(name.to_string(), "d1".to_string())
})?,
d2: vm.get_integer(addr + 2).map_err(|_| {
d2: vm.get_integer((addr + 2)?).map_err(|_| {
HintError::IdentifierHasNoMember(name.to_string(), "d2".to_string())
})?,
})
Expand Down Expand Up @@ -91,7 +91,7 @@ pub fn bigint_to_uint256(
) -> Result<(), HintError> {
let x_struct = get_relocatable_from_var_name("x", vm, ids_data, ap_tracking)?;
let d0 = vm.get_integer(x_struct)?;
let d1 = vm.get_integer(x_struct + 1_i32)?;
let d1 = vm.get_integer((x_struct + 1_i32)?)?;
let d0 = d0.as_ref();
let d1 = d1.as_ref();
let base_86 = constants
Expand Down
4 changes: 2 additions & 2 deletions src/hint_processor/builtin_hint_processor/secp/ec_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl EcPoint<'_> {
let point_addr = get_relocatable_from_var_name(name, vm, ids_data, ap_tracking)?;
Ok(EcPoint {
x: BigInt3::from_base_addr(point_addr, &format!("{}.x", name), vm)?,
y: BigInt3::from_base_addr(point_addr + 3, &format!("{}.y", name), vm)?,
y: BigInt3::from_base_addr((point_addr + 3)?, &format!("{}.y", name), vm)?,
})
}
}
Expand Down Expand Up @@ -71,7 +71,7 @@ pub fn ec_negate(
.to_bigint();

//ids.point
let point_y = get_relocatable_from_var_name("point", vm, ids_data, ap_tracking)? + 3i32;
let point_y = (get_relocatable_from_var_name("point", vm, ids_data, ap_tracking)? + 3i32)?;
let y_bigint3 = BigInt3::from_base_addr(point_y, "point.y", vm)?;
let y = pack(y_bigint3);
let value = (-y).mod_floor(&secp_p);
Expand Down
4 changes: 2 additions & 2 deletions src/hint_processor/builtin_hint_processor/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ pub fn set_add(
));
}

let range_limit = set_end_ptr.sub(&set_ptr)?;
let range_limit = (set_end_ptr - set_ptr)?;

for i in (0..range_limit).step_by(elm_size) {
let set_iter = vm
.get_range(&MaybeRelocatable::from(set_ptr + i), elm_size)
.get_range(&MaybeRelocatable::from((set_ptr + i)?), elm_size)
.map_err(VirtualMachineError::Memory)?;

if set_iter == elm {
Expand Down
2 changes: 1 addition & 1 deletion src/hint_processor/builtin_hint_processor/sha256_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub fn sha256_main(
let mut message: Vec<u8> = Vec::with_capacity(4 * SHA256_INPUT_CHUNK_SIZE_FELTS);

for i in 0..SHA256_INPUT_CHUNK_SIZE_FELTS {
let input_element = vm.get_integer(input_ptr + i)?;
let input_element = vm.get_integer((input_ptr + i)?)?;
let bytes = felt_to_u32(input_element.as_ref())?.to_be_bytes();
message.extend(bytes);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub fn squash_dict_inner_continue_loop(
};
//loop_temps.delta_minus1 = loop_temps + 3 as it is the fourth field of the struct
//Insert loop_temps.delta_minus1 into memory
let should_continue_addr = loop_temps_addr + 3_i32;
let should_continue_addr = (loop_temps_addr + 3_i32)?;
vm.insert_value(should_continue_addr, should_continue)
.map_err(HintError::Memory)
}
Expand Down Expand Up @@ -265,7 +265,7 @@ pub fn squash_dict(
//A map from key to the list of indices accessing it.
let mut access_indices = HashMap::<Felt, Vec<Felt>>::new();
for i in 0..n_accesses_usize {
let key_addr = address + DICT_ACCESS_SIZE * i;
let key_addr = (address + DICT_ACCESS_SIZE * i)?;
let key = vm
.get_integer(key_addr)
.map_err(|_| MemoryError::ExpectedInteger(key_addr))?;
Expand Down
18 changes: 9 additions & 9 deletions src/hint_processor/builtin_hint_processor/uint256_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ pub fn uint256_add(
let a_relocatable = get_relocatable_from_var_name("a", vm, ids_data, ap_tracking)?;
let b_relocatable = get_relocatable_from_var_name("b", vm, ids_data, ap_tracking)?;
let a_low = vm.get_integer(a_relocatable)?;
let a_high = vm.get_integer(a_relocatable + 1_usize)?;
let a_high = vm.get_integer((a_relocatable + 1_usize)?)?;
let b_low = vm.get_integer(b_relocatable)?;
let b_high = vm.get_integer(b_relocatable + 1_usize)?;
let b_high = vm.get_integer((b_relocatable + 1_usize)?)?;
let a_low = a_low.as_ref();
let a_high = a_high.as_ref();
let b_low = b_low.as_ref();
Expand Down Expand Up @@ -105,7 +105,7 @@ pub fn uint256_sqrt(
let n_addr = get_relocatable_from_var_name("n", vm, ids_data, ap_tracking)?;
let root_addr = get_relocatable_from_var_name("root", vm, ids_data, ap_tracking)?;
let n_low = vm.get_integer(n_addr)?;
let n_high = vm.get_integer(n_addr + 1_usize)?;
let n_high = vm.get_integer((n_addr + 1_usize)?)?;
let n_low = n_low.as_ref();
let n_high = n_high.as_ref();

Expand All @@ -127,7 +127,7 @@ pub fn uint256_sqrt(
)));
}
vm.insert_value(root_addr, Felt::new(root))?;
vm.insert_value(root_addr + 1_i32, Felt::zero())
vm.insert_value((root_addr + 1_i32)?, Felt::zero())
.map_err(HintError::Memory)
}

Expand All @@ -141,7 +141,7 @@ pub fn uint256_signed_nn(
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let a_addr = get_relocatable_from_var_name("a", vm, ids_data, ap_tracking)?;
let a_high = vm.get_integer(a_addr + 1_usize)?;
let a_high = vm.get_integer((a_addr + 1_usize)?)?;
//Main logic
//memory[ap] = 1 if 0 <= (ids.a.high % PRIME) < 2 ** 127 else 0
let result: Felt = if !a_high.is_negative() && a_high.as_ref() <= &Felt::new(i128::MAX) {
Expand Down Expand Up @@ -176,9 +176,9 @@ pub fn uint256_unsigned_div_rem(
let remainder_addr = get_relocatable_from_var_name("remainder", vm, ids_data, ap_tracking)?;

let a_low = vm.get_integer(a_addr)?;
let a_high = vm.get_integer(a_addr + 1_usize)?;
let a_high = vm.get_integer((a_addr + 1_usize)?)?;
let div_low = vm.get_integer(div_addr)?;
let div_high = vm.get_integer(div_addr + 1_usize)?;
let div_high = vm.get_integer((div_addr + 1_usize)?)?;
let a_low = a_low.as_ref();
let a_high = a_high.as_ref();
let div_low = div_low.as_ref();
Expand Down Expand Up @@ -208,11 +208,11 @@ pub fn uint256_unsigned_div_rem(
//Insert ids.quotient.low
vm.insert_value(quotient_addr, quotient_low)?;
//Insert ids.quotient.high
vm.insert_value(quotient_addr + 1_i32, quotient_high)?;
vm.insert_value((quotient_addr + 1_i32)?, quotient_high)?;
//Insert ids.remainder.low
vm.insert_value(remainder_addr, remainder_low)?;
//Insert ids.remainder.high
vm.insert_value(remainder_addr + 1_i32, remainder_high)?;
vm.insert_value((remainder_addr + 1_i32)?, remainder_high)?;
Ok(())
}

Expand Down
6 changes: 3 additions & 3 deletions src/hint_processor/builtin_hint_processor/usort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub fn usort_body(
let mut positions_dict: HashMap<Felt, Vec<u64>> = HashMap::new();
let mut output: Vec<Felt> = Vec::new();
for i in 0..input_len_u64 {
let val = vm.get_integer(input_ptr + i as usize)?.into_owned();
let val = vm.get_integer((input_ptr + i as usize)?)?.into_owned();
if let Err(output_index) = output.binary_search(&val) {
output.insert(output_index, val.clone());
}
Expand All @@ -65,11 +65,11 @@ pub fn usort_body(
let output_len = output.len();

for (i, sorted_element) in output.into_iter().enumerate() {
vm.insert_value(output_base + i, sorted_element)?;
vm.insert_value((output_base + i)?, sorted_element)?;
}

for (i, repetition_amount) in multiplicities.into_iter().enumerate() {
vm.insert_value(multiplicities_base + i, Felt::new(repetition_amount))?;
vm.insert_value((multiplicities_base + i)?, Felt::new(repetition_amount))?;
}

insert_value_from_var_name(
Expand Down
10 changes: 5 additions & 5 deletions src/hint_processor/hint_processor_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ pub fn compute_addr_from_reference(
&hint_reference.offset2,
)?;

Some(offset1 + value.get_int_ref()?.to_usize()?)
Some((offset1 + value.get_int_ref()?.to_usize()?).ok()?)
}
OffsetValue::Value(value) => Some(offset1 + *value),
OffsetValue::Value(value) => Some((offset1 + *value).ok()?),
_ => None,
}
}
Expand All @@ -129,7 +129,7 @@ fn apply_ap_tracking_correction(
return None;
}
let ap_diff = hint_ap_tracking.offset - ref_ap_tracking.offset;
ap.sub_usize(ap_diff).ok()
(ap - ap_diff).ok()
}

//Tries to convert a Felt value to usize
Expand Down Expand Up @@ -168,9 +168,9 @@ fn get_offset_value_reference(
}

if *deref {
vm.get_maybe(&(base_addr + *offset))
vm.get_maybe(&(base_addr + *offset).ok()?)
} else {
Some((base_addr + *offset).into())
Some((base_addr + *offset).ok()?.into())
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/types/errors/math_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::types::relocatable::{MaybeRelocatable, Relocatable};

#[derive(Debug, Error, PartialEq)]
pub enum MathError {
// Math functions
#[error("Can't calculate the square root of negative number: {0})")]
SqrtNegative(Felt),
#[error("{0} is not divisible by {1}")]
Expand All @@ -28,7 +29,9 @@ pub enum MathError {
#[error("Operation failed: {0} - {1}, offsets cant be negative")]
RelocatableSubNegOffset(Relocatable, usize),
#[error("Operation failed: {0} + {1}, maximum offset value exceeded")]
RelocatableAddOffsetExceeded(Relocatable, Felt),
RelocatableAddFeltOffsetExceeded(Relocatable, Felt),
#[error("Operation failed: {0} + {1}, maximum offset value exceeded")]
RelocatableAddUsizeOffsetExceeded(Relocatable, usize),
#[error("Operation failed: {0} + {1}, cant add two relocatable values")]
RelocatableAdd(Relocatable, Relocatable),
#[error("Operation failed: {0} - {1}, cant substract two relocatable values with different segment indexes")]
Expand Down
Loading

0 comments on commit 1b8f29e

Please sign in to comment.