From 4ffb4f60ea1c49977c685dddb702445579d1f41c Mon Sep 17 00:00:00 2001 From: Franco Victorio Date: Mon, 13 Jan 2025 11:03:11 +0100 Subject: [PATCH] fix: ignore unknown opcodes in source maps (#764) * fix: ignore unknown opcodes in source maps * Create chatty-roses-itch.md * Stop decoding instructions after finding an invalid opcode * Drop write lock immediately after using it * Qualify log::debug instead of importing it --- .changeset/chatty-roses-itch.md | 5 +++++ Cargo.lock | 1 + crates/edr_solidity/Cargo.toml | 1 + crates/edr_solidity/src/compiler.rs | 26 +++++++++++++++----------- crates/edr_solidity/src/source_map.rs | 11 ++++++++++- 5 files changed, 32 insertions(+), 12 deletions(-) create mode 100644 .changeset/chatty-roses-itch.md diff --git a/.changeset/chatty-roses-itch.md b/.changeset/chatty-roses-itch.md new file mode 100644 index 0000000000..0940ee35de --- /dev/null +++ b/.changeset/chatty-roses-itch.md @@ -0,0 +1,5 @@ +--- +"@nomicfoundation/edr": patch +--- + +fix: ignore unknown opcodes in source maps diff --git a/Cargo.lock b/Cargo.lock index 6059a5ea42..5c3483fed2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1289,6 +1289,7 @@ dependencies = [ "edr_eth", "edr_evm", "indexmap 2.2.6", + "log", "parking_lot 0.12.3", "semver 1.0.23", "serde", diff --git a/crates/edr_solidity/Cargo.toml b/crates/edr_solidity/Cargo.toml index 9b2e11f328..07ae0bc2b2 100644 --- a/crates/edr_solidity/Cargo.toml +++ b/crates/edr_solidity/Cargo.toml @@ -11,6 +11,7 @@ alloy-sol-types = { version = "0.7.4", default-features = false, features = ["st edr_eth = { version = "0.3.5", path = "../edr_eth" } edr_evm = { version = "0.3.5", path = "../edr_evm" } indexmap = { version = "2", features = ["serde"] } +log = { version = "0.4.17", default-features = false } parking_lot = { version = "0.12.1", default-features = false } serde = { version = "1.0.158", default-features = false, features = ["std"] } serde_json = { version = "1.0.89", features = ["preserve_order"] } diff --git a/crates/edr_solidity/src/compiler.rs b/crates/edr_solidity/src/compiler.rs index ca13a1a4da..456d6d147e 100644 --- a/crates/edr_solidity/src/compiler.rs +++ b/crates/edr_solidity/src/compiler.rs @@ -709,19 +709,23 @@ fn decode_bytecodes( for contract in build_model.contract_id_to_contract.values() { let contract_rc = contract.clone(); - let mut contract = contract.write(); - - let contract_file = &contract.location.file().read().source_name.clone(); - let contract_evm_output = &compiler_output.contracts[contract_file][&contract.name].evm; - let contract_abi_output = &compiler_output.contracts[contract_file][&contract.name].abi; - - for item in contract_abi_output { - if item.r#type.as_deref() == Some("error") { - if let Ok(custom_error) = CustomError::from_abi(item.clone()) { - contract.add_custom_error(custom_error); + let contract_evm_output = { + let mut contract = contract.write(); + + let contract_file = &contract.location.file().read().source_name.clone(); + let contract_evm_output = &compiler_output.contracts[contract_file][&contract.name].evm; + let contract_abi_output = &compiler_output.contracts[contract_file][&contract.name].abi; + + for item in contract_abi_output { + if item.r#type.as_deref() == Some("error") { + if let Ok(custom_error) = CustomError::from_abi(item.clone()) { + contract.add_custom_error(custom_error); + } } } - } + + contract_evm_output + }; // This is an abstract contract if contract_evm_output.bytecode.object.is_empty() { diff --git a/crates/edr_solidity/src/source_map.rs b/crates/edr_solidity/src/source_map.rs index 8346f2d00b..7f86c904ea 100644 --- a/crates/edr_solidity/src/source_map.rs +++ b/crates/edr_solidity/src/source_map.rs @@ -159,7 +159,16 @@ pub fn decode_instructions( let source_map = &source_maps[instructions.len()]; let pc = bytes_index; - let opcode = OpCode::new(bytecode[pc]).expect("Invalid opcode"); + let opcode = if let Some(opcode) = OpCode::new(bytecode[pc]) { + opcode + } else { + log::debug!("Invalid opcode {} at pc: {}", bytecode[pc], pc); + + // We assume this happens because the source maps point to the metadata region + // of the bytecode. That means that the actual instructions have + // already been decoded and we can stop here. + return instructions; + }; let push_data = if opcode.is_push() { let push_data = &bytecode[bytes_index..][..1 + opcode.info().immediate_size() as usize];