Skip to content

Commit

Permalink
Revert with proper panic if array index out of bounds (#606)
Browse files Browse the repository at this point in the history
  • Loading branch information
cburgdorf authored Dec 11, 2021
1 parent 86aee21 commit ada8827
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 1 deletion.
9 changes: 9 additions & 0 deletions crates/test-files/fixtures/features/arrays.fe
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
contract Foo:
my_array: Array<u256, 10>

pub fn get_from_storage(self, index: u256) -> u256:
return self.my_array[index]

pub fn get_from_memory(index: u256) -> u256:
let my_array: Array<u256, 10> = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
return my_array[index]
4 changes: 4 additions & 0 deletions crates/test-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ pub fn encoded_over_or_underflow() -> Vec<u8> {
encode_revert("Panic(uint256)", &[uint_token(0x11)])
}

pub fn encoded_panic_out_of_bounds() -> Vec<u8> {
encode_revert("Panic(uint256)", &[uint_token(0x32)])
}

pub fn encoded_div_or_mod_by_zero() -> Vec<u8> {
encode_revert("Panic(uint256)", &[uint_token(0x12)])
}
Expand Down
31 changes: 31 additions & 0 deletions crates/tests/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,37 @@ fn test_assert() {
})
}

#[test]
fn test_arrays() {
with_executor(&|mut executor| {
let harness = deploy_contract(&mut executor, "arrays.fe", "Foo", &[]);

harness.test_function(
&mut executor,
"get_from_memory",
&[uint_token(9)],
Some(&uint_token(10)),
);

validate_revert(
harness.capture_call(&mut executor, "get_from_memory", &[uint_token(10)]),
&encoded_panic_out_of_bounds(),
);

harness.test_function(
&mut executor,
"get_from_storage",
&[uint_token(9)],
Some(&uint_token(0)),
);

validate_revert(
harness.capture_call(&mut executor, "get_from_storage", &[uint_token(10)]),
&encoded_panic_out_of_bounds(),
);
})
}

#[rstest(fixture_file, input, expected,
case("for_loop_with_static_array.fe", &[], uint_token(30)),
case("for_loop_with_static_array_from_sto.fe", &[], uint_token(6)),
Expand Down
1 change: 1 addition & 0 deletions crates/yulgen/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub fn numeric_min_max() -> HashMap<Integer, (yul::Expression, yul::Expression)>
pub const PANIC_FAILED_ASSERTION: usize = 0x01;
pub const PANIC_OVER_OR_UNDERFLOW: usize = 0x11;
pub const PANIC_DIV_OR_MOD_BY_ZERO: usize = 0x12;
pub const PANIC_OUT_OF_BOUNDS: usize = 0x32;

pub const ERROR_INSUFFICIENT_FUNDS_TO_SEND_VALUE: usize = 0x100;
pub const ERROR_FAILED_SEND_VALUE: usize = 0x101;
Expand Down
3 changes: 2 additions & 1 deletion crates/yulgen/src/operations/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,5 +139,6 @@ pub fn indexed_array(
index: yul::Expression,
) -> yul::Expression {
let inner_size = literal_expression! { (typ.inner.size()) };
expression! { add([array], (mul([index], [inner_size]))) }
let array_length = literal_expression! { (typ.size) };
expression! { get_array_item([array], [array_length], [index], [inner_size] ) }
}
17 changes: 17 additions & 0 deletions crates/yulgen/src/runtime/functions/data.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
use crate::constants::PANIC_OUT_OF_BOUNDS;
use crate::operations::revert as revert_operations;

use yultsur::*;

/// Return all data runtime functions
Expand All @@ -15,6 +18,7 @@ pub fn all() -> Vec<yul::Statement> {
ceil32(),
cloadn(),
free(),
get_array_item(),
load_data_string(),
map_value_ptr(),
mcopym(),
Expand Down Expand Up @@ -366,3 +370,16 @@ pub fn load_data_string() -> yul::Statement {
}
}
}

/// Returns a pointer to the array item at the requested index.
/// Reverts with a panic if the index is out of bounds.
pub fn get_array_item() -> yul::Statement {
function_definition! {
function get_array_item(array_ptr, array_length, index, inner_size) -> ptr {
(if (iszero((lt(index, array_length)))) {
[revert_operations::panic_revert(PANIC_OUT_OF_BOUNDS)]
} )
(ptr := add(array_ptr, (mul(index, inner_size))))
}
}
}
3 changes: 3 additions & 0 deletions newsfragments/606.feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Added an out of bounds check for accessing array items.
If an array index is retrieved at an index that is not within
the bounds of the array it now reverts with `Panic(0x32)`.

0 comments on commit ada8827

Please sign in to comment.