diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp index 88ef959bec41..0549c73423c9 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp @@ -12,6 +12,7 @@ enum class AvmError : uint32_t { INVALID_TAG_VALUE, CHECK_TAG_ERROR, ADDR_RES_TAG_ERROR, + MEM_SLICE_OUT_OF_RANGE, REL_ADDR_OUT_OF_RANGE, DIV_ZERO, PARSING_ERROR, diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index 367e854a9107..c554799d18ac 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -142,6 +142,13 @@ bool is_canonical(FF contract_address) contract_address == FEE_JUICE_ADDRESS || contract_address == ROUTER_ADDRESS; } +// Returns true if the slice is not out of memory. +inline bool check_slice_mem_range(uint32_t start_addr, uint32_t size) +{ + // Detect overflow for the highest address (start_addr + size - 1) in the slice. + return !(size > 1 && start_addr + size - 1 < start_addr); +} + } // anonymous namespace /************************************************************************************************** @@ -517,7 +524,7 @@ bool AvmTraceBuilder::check_tag(AvmMemoryTag tag, AddressWithMode addr) return unconstrained_get_memory_tag(addr) == tag; } -bool AvmTraceBuilder::check_tag_range(AvmMemoryTag tag, AddressWithMode start_offset, uint32_t size) +bool AvmTraceBuilder::check_tag_range(AvmMemoryTag tag, uint32_t start_offset, uint32_t size) { for (uint32_t i = 0; i < size; i++) { if (!check_tag(tag, start_offset + i)) { @@ -548,27 +555,30 @@ void AvmTraceBuilder::write_to_memory(AddressWithMode addr, FF val, AvmMemoryTag } template -void AvmTraceBuilder::read_slice_from_memory(AddressWithMode addr, size_t slice_len, std::vector& slice) +AvmError AvmTraceBuilder::read_slice_from_memory(uint32_t addr, uint32_t slice_len, std::vector& slice) { - uint32_t base_addr = addr.offset; - if (addr.mode == AddressingMode::INDIRECT) { - base_addr = static_cast(mem_trace_builder.unconstrained_read(call_ptr, base_addr)); + if (!check_slice_mem_range(addr, slice_len)) { + return AvmError::MEM_SLICE_OUT_OF_RANGE; } for (uint32_t i = 0; i < slice_len; i++) { - slice.push_back(static_cast(mem_trace_builder.unconstrained_read(call_ptr, base_addr + i))); + slice.push_back(static_cast(mem_trace_builder.unconstrained_read(call_ptr, addr + i))); } + return AvmError::NO_ERROR; } -template -void AvmTraceBuilder::write_slice_to_memory(AddressWithMode addr, AvmMemoryTag w_tag, const T& slice) +template AvmError AvmTraceBuilder::write_slice_to_memory(uint32_t addr, AvmMemoryTag w_tag, const T& slice) { - auto base_addr = addr.mode == AddressingMode::INDIRECT - ? static_cast(mem_trace_builder.unconstrained_read(call_ptr, addr.offset)) - : addr.offset; - for (uint32_t i = 0; i < slice.size(); i++) { - write_to_memory(base_addr + i, slice[i], w_tag); + const size_t slice_len = slice.size(); + // Memory out-of-range occurs if slice being huge (more than 32 bits) or highest address overflow. + if (slice_len > UINT32_MAX || !check_slice_mem_range(addr, static_cast(slice_len))) { + return AvmError::MEM_SLICE_OUT_OF_RANGE; + } + + for (uint32_t i = 0; i < slice_len; i++) { + write_to_memory(addr + i, slice[i], w_tag); } + return AvmError::NO_ERROR; } // Finalise Lookup Counts @@ -2150,15 +2160,22 @@ AvmError AvmTraceBuilder::op_calldata_copy(uint8_t indirect, bool is_top_level = current_ext_call_ctx.is_top_level; + // Do not take a reference as calldata might be resized below. auto calldata = current_ext_call_ctx.calldata; if (is_ok(error)) { if (is_top_level) { - slice_trace_builder.create_calldata_copy_slice( - calldata, clk, call_ptr, cd_offset, copy_size, dst_offset_resolved); - mem_trace_builder.write_calldata_copy(calldata, clk, call_ptr, cd_offset, copy_size, dst_offset_resolved); + if (!check_slice_mem_range(dst_offset_resolved, copy_size)) { + error = AvmError::MEM_SLICE_OUT_OF_RANGE; + } else { + slice_trace_builder.create_calldata_copy_slice( + calldata, clk, call_ptr, cd_offset, copy_size, dst_offset_resolved); + mem_trace_builder.write_calldata_copy( + calldata, clk, call_ptr, cd_offset, copy_size, dst_offset_resolved); + } } else { + calldata.resize(copy_size); // If we are not at the top level, we write to memory directly - write_slice_to_memory(dst_offset_resolved, AvmMemoryTag::FF, calldata); + error = write_slice_to_memory(dst_offset_resolved, AvmMemoryTag::FF, calldata); } } @@ -2282,7 +2299,7 @@ AvmError AvmTraceBuilder::op_returndata_copy(uint8_t indirect, // Crucial to perform this operation after having incremented pc because write_slice_to_memory // is implemented with opcodes (SET and JUMP). - write_slice_to_memory(dst_offset_resolved, AvmMemoryTag::FF, returndata_slice); + error = write_slice_to_memory(dst_offset_resolved, AvmMemoryTag::FF, returndata_slice); } return error; } @@ -3529,7 +3546,7 @@ AvmError AvmTraceBuilder::op_get_contract_instance( pc += Deserialization::get_pc_increment(OpCode::GETCONTRACTINSTANCE); - // Crucial to perform this operation after having incremented pc because write_slice_to_memory + // Crucial to perform this operation after having incremented pc because write_to_memory // is implemented with opcodes (SET and JUMP). // TODO(8603): once instructions can have multiple different tags for writes, remove this and do a // constrained writes @@ -3578,7 +3595,6 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log Row row; uint32_t log_size = 0; - AddressWithMode direct_field_addr; uint32_t num_bytes = 0; if (is_ok(error)) { @@ -3592,10 +3608,13 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log std::make_move_iterator(log_size_bytes.begin()), std::make_move_iterator(log_size_bytes.end())); - direct_field_addr = AddressWithMode(static_cast(resolved_log_offset)); - if (!check_tag_range(AvmMemoryTag::FF, direct_field_addr, log_size)) { + if (!check_slice_mem_range(resolved_log_offset, log_size)) { + error = AvmError::MEM_SLICE_OUT_OF_RANGE; + } + + if (is_ok(error) && !check_tag_range(AvmMemoryTag::FF, resolved_log_offset, log_size)) { error = AvmError::CHECK_TAG_ERROR; - }; + } } // Can't return earlier as we do elsewhere for side-effect-limit because we need @@ -3619,7 +3638,7 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log if (is_ok(error)) { // We need to read the rest of the log_size number of elements for (uint32_t i = 0; i < log_size; i++) { - FF log_value = unconstrained_read_from_memory(direct_field_addr + i); + FF log_value = unconstrained_read_from_memory(resolved_log_offset + i); std::vector log_value_byte = log_value.to_buffer(); bytes_to_hash.insert(bytes_to_hash.end(), std::make_move_iterator(log_value_byte.begin()), @@ -3822,7 +3841,15 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode, // Ext Ctx setup std::vector calldata; - read_slice_from_memory(resolved_args_offset, args_size, calldata); + const auto slice_err = read_slice_from_memory(resolved_args_offset, args_size, calldata); + + if (!is_ok(slice_err)) { + return slice_err; + } + + if (!check_tag_range(AvmMemoryTag::FF, resolved_args_offset, args_size)) { + return AvmError::CHECK_TAG_ERROR; + } // Don't try allocating more than the gas that is actually left const auto l2_gas_allocated_to_nested_call = @@ -3985,7 +4012,12 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset const auto da_gas_allocated_to_nested_call = current_ext_call_ctx.start_da_gas_left; // We are returning from a nested call std::vector returndata{}; - read_slice_from_memory(resolved_ret_offset, ret_size, returndata); + const auto slice_err = read_slice_from_memory(resolved_ret_offset, ret_size, returndata); + + if (is_ok(error)) { + error = slice_err; + } + // Pop the caller/parent's context from the stack to proceed with execution there current_ext_call_ctx = external_call_ctx_stack.top(); external_call_ctx_stack.pop(); @@ -4124,7 +4156,12 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset const auto da_gas_allocated_to_nested_call = current_ext_call_ctx.start_da_gas_left; // We are returning from a nested call std::vector returndata{}; - read_slice_from_memory(resolved_ret_offset, ret_size, returndata); + const auto slice_err = read_slice_from_memory(resolved_ret_offset, ret_size, returndata); + + if (is_ok(error)) { + error = slice_err; + } + // Pop the caller/parent's context from the stack to proceed with execution there current_ext_call_ctx = external_call_ctx_stack.top(); external_call_ctx_stack.pop(); @@ -4229,10 +4266,16 @@ AvmError AvmTraceBuilder::op_debug_log(uint8_t indirect, // Constrain gas cost bool out_of_gas = gas_trace_builder.constrain_gas(clk, OpCode::DEBUGLOG, message_size + fields_size); + if (out_of_gas && is_ok(error)) { error = AvmError::OUT_OF_GAS; } + if (is_ok(error) && !(check_slice_mem_range(resolved_message_offset, message_size) && + check_slice_mem_range(resolved_fields_offset, fields_size))) { + error = AvmError::MEM_SLICE_OUT_OF_RANGE; + } + if (is_ok(error) && !(check_tag_range(AvmMemoryTag::U8, resolved_message_offset, message_size) && check_tag_range(AvmMemoryTag::FF, resolved_fields_offset, fields_size))) { error = AvmError::CHECK_TAG_ERROR; @@ -4286,6 +4329,10 @@ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t in error = AvmError::OUT_OF_GAS; } + if (is_ok(error) && !check_slice_mem_range(resolved_input_offset, 4)) { + error = AvmError::MEM_SLICE_OUT_OF_RANGE; + } + // These read patterns will be refactored - we perform them here instead of in the poseidon gadget trace // even though they are "performed" by the gadget. AddressWithMode direct_src_offset = { AddressingMode::DIRECT, resolved_input_offset }; @@ -4319,12 +4366,16 @@ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t in IntermRegister::ID, AvmMemTraceBuilder::POSEIDON2); - bool read_tag_valid = read_a.tag_match && read_b.tag_match && read_c.tag_match && read_d.tag_match; + const bool read_tag_valid = read_a.tag_match && read_b.tag_match && read_c.tag_match && read_d.tag_match; if (is_ok(error) && !read_tag_valid) { error = AvmError::CHECK_TAG_ERROR; } + if (is_ok(error) && !check_slice_mem_range(resolved_output_offset, 4)) { + error = AvmError::MEM_SLICE_OUT_OF_RANGE; + } + if (is_ok(error)) { std::array input = { read_a.val, read_b.val, read_c.val, read_d.val }; std::array result = poseidon2_trace_builder.poseidon2_permutation( @@ -4430,8 +4481,13 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, call_ptr, clk, resolved_inputs_offset, AvmMemoryTag::U32, AvmMemoryTag::FF, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_ok(error) && !(check_tag_range(AvmMemoryTag::U32, resolved_state_offset, STATE_SIZE) && - check_tag_range(AvmMemoryTag::U32, resolved_inputs_offset, INPUTS_SIZE))) { + if (is_ok(error) && !(check_slice_mem_range(resolved_inputs_offset, INPUTS_SIZE)) && + check_slice_mem_range(resolved_state_offset, STATE_SIZE)) { + error = AvmError::MEM_SLICE_OUT_OF_RANGE; + } + + if (is_ok(error) && !(check_tag_range(AvmMemoryTag::U32, resolved_inputs_offset, INPUTS_SIZE) && + check_tag_range(AvmMemoryTag::U32, resolved_state_offset, STATE_SIZE))) { error = AvmError::CHECK_TAG_ERROR; } @@ -4483,20 +4539,27 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, // Input for hash is expanded to 512 bits std::vector input_vec; // Read results are written to h_init array. - read_slice_from_memory(resolved_state_offset, 8, h_init_vec); + auto slice_err = read_slice_from_memory(resolved_state_offset, STATE_SIZE, h_init_vec); + + // This slice is not out of memory range, otherwise an error would be returned above. (check_slice_mem_range()) + ASSERT(slice_err == AvmError::NO_ERROR); + // Read results are written to input array - read_slice_from_memory(resolved_inputs_offset, 16, input_vec); + slice_err = read_slice_from_memory(resolved_inputs_offset, INPUTS_SIZE, input_vec); + + // This slice is not out of memory range, otherwise an error would be returned above. (check_slice_mem_range()) + ASSERT(slice_err == AvmError::NO_ERROR); // Now that we have read all the values, we can perform the operation to get the resulting witness. // Note: We use the sha_op_clk to ensure that the sha256 operation is performed at the same clock cycle as the // main trace that has the selector - std::array h_init = vec_to_arr(h_init_vec); - std::array input = vec_to_arr(input_vec); + std::array h_init = vec_to_arr(h_init_vec); + std::array input = vec_to_arr(input_vec); - std::array result = sha256_trace_builder.sha256_compression(h_init, input, sha_op_clk); + std::array result = sha256_trace_builder.sha256_compression(h_init, input, sha_op_clk); // We convert the results to field elements here std::vector ff_result; - for (uint32_t i = 0; i < 8; i++) { + for (uint32_t i = 0; i < STATE_SIZE; i++) { ff_result.emplace_back(result[i]); } @@ -4504,7 +4567,7 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, // Crucial to perform this operation after having incremented pc because write_slice_to_memory // is implemented with opcodes (SET and JUMP). - write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U32, ff_result); + error = write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U32, ff_result); return AvmError::NO_ERROR; } @@ -4532,8 +4595,15 @@ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offse call_ptr, clk, resolved_input_offset, AvmMemoryTag::U64, AvmMemoryTag::FF, IntermRegister::IA); bool tag_match = input_read.tag_match; - if (is_ok(error) && - !(tag_match && check_tag_range(AvmMemoryTag::U64, resolved_input_offset, KECCAKF1600_INPUT_SIZE))) { + if (is_ok(error) && !tag_match) { + error = AvmError::CHECK_TAG_ERROR; + } + + if (is_ok(error) && !check_slice_mem_range(resolved_input_offset, KECCAKF1600_INPUT_SIZE)) { + error = AvmError::MEM_SLICE_OUT_OF_RANGE; + } + + if (is_ok(error) && !check_tag_range(AvmMemoryTag::U64, resolved_input_offset, KECCAKF1600_INPUT_SIZE)) { error = AvmError::CHECK_TAG_ERROR; } @@ -4565,7 +4635,11 @@ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offse // Array input is fixed to 1600 bits std::vector input_vec; // Read results are written to input array - read_slice_from_memory(resolved_input_offset, KECCAKF1600_INPUT_SIZE, input_vec); + const auto slice_err = read_slice_from_memory(resolved_input_offset, KECCAKF1600_INPUT_SIZE, input_vec); + + // This slice is not out of memory range, otherwise an error would be returned above. (check_slice_mem_range()) + ASSERT(slice_err == AvmError::NO_ERROR); + std::array input = vec_to_arr(input_vec); // Now that we have read all the values, we can perform the operation to get the resulting witness. @@ -4577,9 +4651,9 @@ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offse // Crucial to perform this operation after having incremented pc because write_slice_to_memory // is implemented with opcodes (SET and JUMP). - write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U64, result); + error = write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U64, result); - return AvmError::NO_ERROR; + return error; } AvmError AvmTraceBuilder::op_ec_add(uint16_t indirect, @@ -4669,7 +4743,11 @@ AvmError AvmTraceBuilder::op_ec_add(uint16_t indirect, pc += Deserialization::get_pc_increment(OpCode::ECADD); - // Crucial to perform this operation after having incremented pc because write_slice_to_memory + if (!check_slice_mem_range(resolved_output_offset, 3)) { + return AvmError::MEM_SLICE_OUT_OF_RANGE; + } + + // Crucial to perform this operation after having incremented pc because write_to_memory // is implemented with opcodes (SET and JUMP). // Write point coordinates write_to_memory(resolved_output_offset, result.x, AvmMemoryTag::FF); @@ -4702,6 +4780,10 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, const FF points_length = is_ok(error) ? unconstrained_read_from_memory(resolved_point_length_offset) : 0; + if (is_ok(error) && !check_slice_mem_range(resolved_points_offset, static_cast(points_length))) { + error = AvmError::MEM_SLICE_OUT_OF_RANGE; + } + // Points are stored as [x1, y1, inf1, x2, y2, inf2, ...] with the types [FF, FF, U8, FF, FF, U8, ...] const uint32_t num_points = uint32_t(points_length) / 3; // 3 elements per point // We need to split up the reads due to the memory tags, @@ -4709,18 +4791,24 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, std::vector points_inf_vec; std::vector scalars_vec; - bool tags_valid = true; for (uint32_t i = 0; i < num_points; i++) { - tags_valid = tags_valid && check_tag_range(AvmMemoryTag::FF, resolved_points_offset + 3 * i, 2) && - check_tag(AvmMemoryTag::U1, resolved_points_offset + 3 * i + 2); + if (is_ok(error) && !check_tag_range(AvmMemoryTag::FF, resolved_points_offset + 3 * i, 2)) { + error = AvmError::CHECK_TAG_ERROR; + } + + if (is_ok(error) && !check_tag(AvmMemoryTag::U1, resolved_points_offset + 3 * i + 2)) { + error = AvmError::CHECK_TAG_ERROR; + } } // Scalar read length is num_points* 2 since scalars are stored as lo and hi limbs uint32_t scalar_read_length = num_points * 2; - tags_valid = tags_valid && check_tag_range(AvmMemoryTag::FF, resolved_scalars_offset, scalar_read_length); + if (is_ok(error) && !check_slice_mem_range(resolved_scalars_offset, static_cast(scalar_read_length))) { + error = AvmError::MEM_SLICE_OUT_OF_RANGE; + } - if (is_ok(error) && !tags_valid) { + if (is_ok(error) && !check_tag_range(AvmMemoryTag::FF, resolved_scalars_offset, scalar_read_length)) { error = AvmError::CHECK_TAG_ERROR; } @@ -4760,7 +4848,10 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, // Scalars are easy to read since they are stored as [lo1, hi1, lo2, hi2, ...] with the types [FF, FF, FF,FF, // ...] - read_slice_from_memory(resolved_scalars_offset, scalar_read_length, scalars_vec); + const auto slice_err = read_slice_from_memory(resolved_scalars_offset, scalar_read_length, scalars_vec); + + // This slice is not out of memory range, otherwise an error would be returned above. (check_slice_mem_range()) + ASSERT(slice_err == AvmError::NO_ERROR); // Reconstruct Grumpkin points std::vector points; @@ -4797,7 +4888,11 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, pc += Deserialization::get_pc_increment(OpCode::MSM); - // Crucial to perform this operation after having incremented pc because write_slice_to_memory + if (!check_slice_mem_range(resolved_output_offset, 3)) { + return AvmError::MEM_SLICE_OUT_OF_RANGE; + } + + // Crucial to perform this operation after having incremented pc because write_to_memory // is implemented with opcodes (SET and JUMP). // Write the result back to memory [x, y, inf] with tags [FF, FF, U8] write_to_memory(resolved_output_offset, result.x, AvmMemoryTag::FF); @@ -4926,7 +5021,12 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint16_t indirect, // Crucial to perform this operation after having incremented pc because write_slice_to_memory // is implemented with opcodes (SET and JUMP). - write_slice_to_memory(resolved_dst_offset, w_in_tag, res); + const auto slice_err = write_slice_to_memory(resolved_dst_offset, w_in_tag, res); + + if (is_ok(error)) { + error = slice_err; + } + return error; } @@ -5217,8 +5317,8 @@ std::vector AvmTraceBuilder::finalize(bool apply_end_gas_assertions) public_inputs.gas_settings.gas_limits.l2_gas - public_inputs.start_gas_used.l2_gas; auto const allocated_l2_gas = std::min(l2_gas_left_after_private, static_cast(MAX_L2_GAS_PER_TX_PUBLIC_PORTION)); - // Since end_gas_used also contains start_gas_used, we need to subtract it out to see what is consumed by the - // public computation only + // Since end_gas_used also contains start_gas_used, we need to subtract it out to see what is consumed by + // the public computation only auto expected_public_gas_consumed = public_inputs.end_gas_used.l2_gas - public_inputs.start_gas_used.l2_gas; auto expected_end_gas_l2 = allocated_l2_gas - expected_public_gas_consumed; auto last_l2_gas_remaining = main_trace.back().main_l2_gas_remaining; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp index f228501aa143..0ae4e06d904f 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp @@ -400,11 +400,11 @@ class AvmTraceBuilder { // TODO: remove these once everything is constrained. AvmMemoryTag unconstrained_get_memory_tag(AddressWithMode addr); bool check_tag(AvmMemoryTag tag, AddressWithMode addr); - bool check_tag_range(AvmMemoryTag tag, AddressWithMode start_offset, uint32_t size); + bool check_tag_range(AvmMemoryTag tag, uint32_t start_offset, uint32_t size); FF unconstrained_read_from_memory(AddressWithMode addr); - template void read_slice_from_memory(AddressWithMode addr, size_t slice_len, std::vector& slice); + template AvmError read_slice_from_memory(uint32_t addr, uint32_t slice_len, std::vector& slice); void write_to_memory(AddressWithMode addr, FF val, AvmMemoryTag w_tag, bool fix_pc = true); - template void write_slice_to_memory(AddressWithMode addr, AvmMemoryTag w_tag, const T& slice); + template AvmError write_slice_to_memory(uint32_t addr, AvmMemoryTag w_tag, const T& slice); }; } // namespace bb::avm_trace diff --git a/yarn-project/simulator/src/avm/avm_memory_types.ts b/yarn-project/simulator/src/avm/avm_memory_types.ts index a1c86f870b11..9b3dd8d64eb2 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.ts @@ -15,7 +15,12 @@ import { type FunctionsOf } from '@aztec/foundation/types'; import { strict as assert } from 'assert'; -import { InstructionExecutionError, InvalidTagValueError, TagCheckError } from './errors.js'; +import { + InstructionExecutionError, + InvalidTagValueError, + MemorySliceOutOfRangeError, + TagCheckError, +} from './errors.js'; import { Addressing, AddressingMode } from './opcodes/addressing_mode.js'; /** MemoryValue gathers the common operations for all memory types. */ @@ -271,7 +276,11 @@ export class TaggedMemory implements TaggedMemoryInterface { public getSlice(offset: number, size: number): MemoryValue[] { assert(Number.isInteger(offset) && Number.isInteger(size)); - assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE); + + if (offset + size > TaggedMemory.MAX_MEMORY_SIZE) { + throw new MemorySliceOutOfRangeError(offset, size); + } + const slice = new Array(size); for (let i = 0; i < size; i++) { @@ -299,7 +308,11 @@ export class TaggedMemory implements TaggedMemoryInterface { public setSlice(offset: number, slice: MemoryValue[]) { assert(Number.isInteger(offset)); - assert(offset + slice.length <= TaggedMemory.MAX_MEMORY_SIZE); + + if (offset + slice.length > TaggedMemory.MAX_MEMORY_SIZE) { + throw new MemorySliceOutOfRangeError(offset, slice.length); + } + slice.forEach((element, idx) => { this._mem.set(offset + idx, element); }); diff --git a/yarn-project/simulator/src/avm/errors.ts b/yarn-project/simulator/src/avm/errors.ts index 25a306fe09cc..30c1fd4726d3 100644 --- a/yarn-project/simulator/src/avm/errors.ts +++ b/yarn-project/simulator/src/avm/errors.ts @@ -102,10 +102,21 @@ export class TagCheckError extends AvmExecutionError { * Error is thrown when a relative memory address resolved to an offset which * is out of range, i.e, greater than maxUint32. */ -export class AddressOutOfRangeError extends AvmExecutionError { +export class RelativeAddressOutOfRangeError extends AvmExecutionError { constructor(baseAddr: number, relOffset: number) { super(`Address out of range. Base address ${baseAddr}, relative offset ${relOffset}`); - this.name = 'AddressOutOfRangeError'; + this.name = 'RelativeAddressOutOfRangeError'; + } +} + +/** + * Error is thrown when a memory slice contains addresses which are + * out of range, i.e, greater than maxUint32. + */ +export class MemorySliceOutOfRangeError extends AvmExecutionError { + constructor(baseAddr: number, size: number) { + super(`Memory slice is out of range. Base address ${baseAddr}, size ${size}`); + this.name = 'MemorySliceOutOfRangeError'; } } diff --git a/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts index 89107196cf9b..c7473bcaf8d6 100644 --- a/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts +++ b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts @@ -1,7 +1,7 @@ import { strict as assert } from 'assert'; import { TaggedMemory, type TaggedMemoryInterface } from '../avm_memory_types.js'; -import { AddressOutOfRangeError } from '../errors.js'; +import { RelativeAddressOutOfRangeError } from '../errors.js'; export enum AddressingMode { DIRECT = 0, @@ -67,7 +67,7 @@ export class Addressing { const baseAddr = Number(mem.get(0).toBigInt()); resolved[i] += baseAddr; if (resolved[i] >= TaggedMemory.MAX_MEMORY_SIZE) { - throw new AddressOutOfRangeError(baseAddr, offset); + throw new RelativeAddressOutOfRangeError(baseAddr, offset); } } if (mode & AddressingMode.INDIRECT) { diff --git a/yarn-project/simulator/src/avm/opcodes/ec_add.ts b/yarn-project/simulator/src/avm/opcodes/ec_add.ts index 0f29954a9673..c4d3dd33e6a5 100644 --- a/yarn-project/simulator/src/avm/opcodes/ec_add.ts +++ b/yarn-project/simulator/src/avm/opcodes/ec_add.ts @@ -84,10 +84,11 @@ export class EcAdd extends Instruction { dest = grumpkin.add(p1, p2); } - memory.set(dstOffset, new Field(dest.x)); - memory.set(dstOffset + 1, new Field(dest.y)); + // Important to use setSlice() and not set() in the two following statements as + // this checks that the offsets lie within memory range. + memory.setSlice(dstOffset, [new Field(dest.x), new Field(dest.y)]); // Check representation of infinity for grumpkin - memory.set(dstOffset + 2, new Uint1(dest.equals(Point.ZERO) ? 1 : 0)); + memory.setSlice(dstOffset + 2, [new Uint1(dest.equals(Point.ZERO) ? 1 : 0)]); memory.assert({ reads: 6, writes: 3, addressing }); } diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.ts index b774c7f4cfc8..22cd3c12385d 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -39,10 +39,10 @@ abstract class ExternalCall extends Instruction { memory.checkTag(TypeTag.UINT32, argsSizeOffset); const calldataSize = memory.get(argsSizeOffset).toNumber(); + const calldata = memory.getSlice(argsOffset, calldataSize).map(f => f.toFr()); memory.checkTagsRange(TypeTag.FIELD, argsOffset, calldataSize); const callAddress = memory.getAs(addrOffset); - const calldata = memory.getSlice(argsOffset, calldataSize).map(f => f.toFr()); // If we are already in a static call, we propagate the environment. const callType = context.environment.isStaticCall ? 'STATICCALL' : this.type; diff --git a/yarn-project/simulator/src/avm/opcodes/hashing.test.ts b/yarn-project/simulator/src/avm/opcodes/hashing.test.ts index ca89cbee50f7..a6f04f3ab131 100644 --- a/yarn-project/simulator/src/avm/opcodes/hashing.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/hashing.test.ts @@ -1,7 +1,8 @@ import { keccakf1600, sha256Compression } from '@aztec/foundation/crypto'; import { type AvmContext } from '../avm_context.js'; -import { Field, Uint32, Uint64 } from '../avm_memory_types.js'; +import { Field, TaggedMemory, Uint32, Uint64 } from '../avm_memory_types.js'; +import { MemorySliceOutOfRangeError } from '../errors.js'; import { initContext, randomMemoryUint32s } from '../fixtures/index.js'; import { Addressing, AddressingMode } from './addressing_mode.js'; import { KeccakF1600, Poseidon2, Sha256Compression } from './hashing.js'; @@ -65,6 +66,26 @@ describe('Hashing Opcodes', () => { new Field(0x16c877b5b9c04d873218804ccbf65d0eeb12db447f66c9ca26fec380055df7e9n), ]); }); + + it('Should throw an error when input state offsets are out of range.', async () => { + const indirect = 0; + const inputStateOffset = TaggedMemory.MAX_MEMORY_SIZE - 3; + const outputStateOffset = 0; + + await expect(new Poseidon2(indirect, inputStateOffset, outputStateOffset).execute(context)).rejects.toThrow( + MemorySliceOutOfRangeError, + ); + }); + + it('Should throw an error when output state offsets are out of range.', async () => { + const indirect = 0; + const inputStateOffset = 0; + const outputStateOffset = TaggedMemory.MAX_MEMORY_SIZE - 1; + + await expect(new Poseidon2(indirect, inputStateOffset, outputStateOffset).execute(context)).rejects.toThrow( + MemorySliceOutOfRangeError, + ); + }); }); describe('Keccakf1600', () => { @@ -94,6 +115,29 @@ describe('Hashing Opcodes', () => { const result = context.machineState.memory.getSliceAs(dstOffset, 25); expect(result).toEqual(keccakf1600(rawArgs).map(a => new Uint64(a))); }); + + it('Should throw an error when message input offsets are out of range.', async () => { + const indirect = 0; + const messageOffset = TaggedMemory.MAX_MEMORY_SIZE - 24; + const dstOffset = 200; + + await expect(new KeccakF1600(indirect, dstOffset, messageOffset).execute(context)).rejects.toThrow( + MemorySliceOutOfRangeError, + ); + }); + + it('Should throw an error when destination offsets are out of range.', async () => { + const rawArgs = [...Array(25)].map(_ => 0n); + const args = rawArgs.map(a => new Uint64(a)); + const indirect = 0; + const messageOffset = 0; + const dstOffset = TaggedMemory.MAX_MEMORY_SIZE - 24; + context.machineState.memory.setSlice(messageOffset, args); + + await expect(new KeccakF1600(indirect, dstOffset, messageOffset).execute(context)).rejects.toThrow( + MemorySliceOutOfRangeError, + ); + }); }); describe('Sha256Compression', () => { @@ -179,5 +223,44 @@ describe('Hashing Opcodes', () => { const expectedOutput = sha256Compression(stateArray, inputsArray); expect(outputArray).toEqual(expectedOutput); }); + + it('Should throw an error when input offsets are out of range.', async () => { + const indirect = 0; + const stateOffset = 0; + const inputsOffset = TaggedMemory.MAX_MEMORY_SIZE - 15; + const outputOffset = 300; + + await expect( + new Sha256Compression(indirect, outputOffset, stateOffset, inputsOffset).execute(context), + ).rejects.toThrow(MemorySliceOutOfRangeError); + }); + + it('Should throw an error when state offsets are out of range.', async () => { + const inputs = randomMemoryUint32s(16); + const indirect = 0; + const stateOffset = TaggedMemory.MAX_MEMORY_SIZE - 7; + const inputsOffset = 200; + const outputOffset = 300; + context.machineState.memory.setSlice(inputsOffset, inputs); + + await expect( + new Sha256Compression(indirect, outputOffset, stateOffset, inputsOffset).execute(context), + ).rejects.toThrow(MemorySliceOutOfRangeError); + }); + + it('Should throw an error when output offsets are out of range.', async () => { + const state = randomMemoryUint32s(8); + const inputs = randomMemoryUint32s(16); + const indirect = 0; + const stateOffset = 0; + const inputsOffset = 200; + const outputOffset = TaggedMemory.MAX_MEMORY_SIZE - 7; + context.machineState.memory.setSlice(stateOffset, state); + context.machineState.memory.setSlice(inputsOffset, inputs); + + await expect( + new Sha256Compression(indirect, outputOffset, stateOffset, inputsOffset).execute(context), + ).rejects.toThrow(MemorySliceOutOfRangeError); + }); }); }); diff --git a/yarn-project/simulator/src/avm/opcodes/hashing.ts b/yarn-project/simulator/src/avm/opcodes/hashing.ts index 2f817814b0ea..1afde431e46e 100644 --- a/yarn-project/simulator/src/avm/opcodes/hashing.ts +++ b/yarn-project/simulator/src/avm/opcodes/hashing.ts @@ -30,9 +30,10 @@ export class Poseidon2 extends Instruction { const operands = [this.inputStateOffset, this.outputStateOffset]; const addressing = Addressing.fromWire(this.indirect, operands.length); const [inputOffset, outputOffset] = addressing.resolve(operands, memory); - memory.checkTagsRange(TypeTag.FIELD, inputOffset, Poseidon2.stateSize); const inputState = memory.getSlice(inputOffset, Poseidon2.stateSize); + memory.checkTagsRange(TypeTag.FIELD, inputOffset, Poseidon2.stateSize); + const outputState = poseidon2Permutation(inputState); memory.setSlice( outputOffset, @@ -68,9 +69,9 @@ export class KeccakF1600 extends Instruction { const [dstOffset, inputOffset] = addressing.resolve(operands, memory); context.machineState.consumeGas(this.gasCost()); + const stateData = memory.getSlice(inputOffset, inputSize).map(word => word.toBigInt()); memory.checkTagsRange(TypeTag.UINT64, inputOffset, inputSize); - const stateData = memory.getSlice(inputOffset, inputSize).map(word => word.toBigInt()); const updatedState = keccakf1600(stateData); const res = updatedState.map(word => new Uint64(word)); @@ -113,11 +114,12 @@ export class Sha256Compression extends Instruction { // Note: size of output is same as size of state context.machineState.consumeGas(this.gasCost()); + const inputs = Uint32Array.from(memory.getSlice(inputsOffset, INPUTS_SIZE).map(word => word.toNumber())); + const state = Uint32Array.from(memory.getSlice(stateOffset, STATE_SIZE).map(word => word.toNumber())); + memory.checkTagsRange(TypeTag.UINT32, inputsOffset, INPUTS_SIZE); memory.checkTagsRange(TypeTag.UINT32, stateOffset, STATE_SIZE); - const state = Uint32Array.from(memory.getSlice(stateOffset, STATE_SIZE).map(word => word.toNumber())); - const inputs = Uint32Array.from(memory.getSlice(inputsOffset, INPUTS_SIZE).map(word => word.toNumber())); const output = sha256Compression(state, inputs); // Conversion required from Uint32Array to Uint32[] (can't map directly, need `...`) diff --git a/yarn-project/simulator/src/avm/opcodes/memory.test.ts b/yarn-project/simulator/src/avm/opcodes/memory.test.ts index 3a43ecc2305d..6cd8b7ff7a9f 100644 --- a/yarn-project/simulator/src/avm/opcodes/memory.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/memory.test.ts @@ -1,7 +1,8 @@ import { Fr } from '@aztec/foundation/fields'; import { type AvmContext } from '../avm_context.js'; -import { Field, TypeTag, Uint8, Uint16, Uint32, Uint64, Uint128 } from '../avm_memory_types.js'; +import { Field, TaggedMemory, TypeTag, Uint8, Uint16, Uint32, Uint64, Uint128 } from '../avm_memory_types.js'; +import { MemorySliceOutOfRangeError } from '../errors.js'; import { initContext, initExecutionEnvironment } from '../fixtures/index.js'; import { Opcode } from '../serialization/instruction_serialization.js'; import { Addressing, AddressingMode } from './addressing_mode.js'; @@ -421,7 +422,21 @@ describe('Memory instructions', () => { expect(actual).toEqual([new Field(2), new Field(3)]); }); - // TODO: check bad cases (i.e., out of bounds) + it('Should return error when memory slice calldatacopy target is out-of-range', async () => { + const calldata = [new Fr(1n), new Fr(2n), new Fr(3n)]; + context = initContext({ env: initExecutionEnvironment({ calldata }) }); + context.machineState.memory.set(0, new Uint32(0)); // cdoffset + context.machineState.memory.set(1, new Uint32(3)); // size + + await expect( + new CalldataCopy( + /*indirect=*/ 0, + /*cdOffset=*/ 0, + /*copySize=*/ 1, + /*dstOffset=*/ TaggedMemory.MAX_MEMORY_SIZE - 2, + ).execute(context), + ).rejects.toThrow(MemorySliceOutOfRangeError); + }); }); describe('RETURNDATASIZE', () => { @@ -505,6 +520,20 @@ describe('Memory instructions', () => { expect(actual).toEqual([new Field(2), new Field(3)]); }); - // TODO: check bad cases (i.e., out of bounds) + it('Should return error when memory slice target is out-of-range', async () => { + context = initContext(); + context.machineState.nestedReturndata = [new Fr(1n), new Fr(2n), new Fr(3n)]; + context.machineState.memory.set(0, new Uint32(1)); // rdoffset + context.machineState.memory.set(1, new Uint32(2)); // size + + await expect( + new ReturndataCopy( + /*indirect=*/ 0, + /*rdOffset=*/ 0, + /*copySize=*/ 1, + /*dstOffset=*/ TaggedMemory.MAX_MEMORY_SIZE - 1, + ).execute(context), + ).rejects.toThrow(MemorySliceOutOfRangeError); + }); }); }); diff --git a/yarn-project/simulator/src/avm/opcodes/misc.ts b/yarn-project/simulator/src/avm/opcodes/misc.ts index 13b348588d86..09d095a05ee8 100644 --- a/yarn-project/simulator/src/avm/opcodes/misc.ts +++ b/yarn-project/simulator/src/avm/opcodes/misc.ts @@ -39,14 +39,15 @@ export class DebugLog extends Instruction { memory.checkTag(TypeTag.UINT32, fieldsSizeOffset); const fieldsSize = memory.get(fieldsSizeOffset).toNumber(); + + const rawMessage = memory.getSlice(messageOffset, this.messageSize); + const fields = memory.getSlice(fieldsOffset, fieldsSize); + memory.checkTagsRange(TypeTag.UINT8, messageOffset, this.messageSize); memory.checkTagsRange(TypeTag.FIELD, fieldsOffset, fieldsSize); context.machineState.consumeGas(this.gasCost(this.messageSize + fieldsSize)); - const rawMessage = memory.getSlice(messageOffset, this.messageSize); - const fields = memory.getSlice(fieldsOffset, fieldsSize); - // Interpret str = [u8; N] to string. const messageAsStr = rawMessage.map(field => String.fromCharCode(field.toNumber())).join(''); const formattedStr = applyStringFormatting( diff --git a/yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts b/yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts index a3e24bb48fdf..8939824c4e68 100644 --- a/yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts +++ b/yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts @@ -46,6 +46,12 @@ export class MultiScalarMul extends Instruction { if (pointsReadLength % 3 !== 0) { throw new InstructionExecutionError(`Points vector offset should be a multiple of 3, was ${pointsReadLength}`); } + + // Get the unrolled (x, y, inf) representing the points + // Important to perform this before tag validation, as getSlice() first checks + // that the slice is not out of memory range. This needs to be aligned with circuit. + const pointsVector = memory.getSlice(pointsOffset, pointsReadLength); + // Divide by 3 since each point is represented as a triplet to get the number of points const numPoints = pointsReadLength / 3; // The tag for each triplet will be (Field, Field, Uint8) @@ -56,8 +62,6 @@ export class MultiScalarMul extends Instruction { // Check Uint1 (inf flag) memory.checkTag(TypeTag.UINT1, offset + 2); } - // Get the unrolled (x, y, inf) representing the points - const pointsVector = memory.getSlice(pointsOffset, pointsReadLength); // The size of the scalars vector is twice the NUMBER of points because of the scalar limb decomposition const scalarReadLength = numPoints * 2; @@ -106,10 +110,11 @@ export class MultiScalarMul extends Instruction { } }, grumpkin.mul(firstBaseScalarPair[0], firstBaseScalarPair[1])); - memory.set(outputOffset, new Field(outputPoint.x)); - memory.set(outputOffset + 1, new Field(outputPoint.y)); + // Important to use setSlice() and not set() in the two following statements as + // this checks that the offsets lie within memory range. + memory.setSlice(outputOffset, [new Field(outputPoint.x), new Field(outputPoint.y)]); // Check representation of infinity for grumpkin - memory.set(outputOffset + 2, new Uint1(outputPoint.equals(Point.ZERO) ? 1 : 0)); + memory.setSlice(outputOffset + 2, [new Uint1(outputPoint.equals(Point.ZERO) ? 1 : 0)]); memory.assert({ reads: 1 + pointsReadLength + scalarReadLength /* points and scalars */,