From b2c83b089083e27f4a44721da1bf08673aa8e684 Mon Sep 17 00:00:00 2001 From: mikamikasuki <211269698+mikamikasuki@users.noreply.github.com> Date: Fri, 2 Oct 2026 05:46:10 -0700 Subject: [PATCH 1/2] Handle overlong AML package lengths --- src/aml/mod.rs | 11 ++++++++--- tests/incorrect_package_length.rs | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 tests/incorrect_package_length.rs diff --git a/src/aml/mod.rs b/src/aml/mod.rs index b013a195..591f8b72 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -777,12 +777,17 @@ where * logic to add some `Uninitialized`s, then go round again to complete * the in-flight operation. * - * To make these consistent, we always remove the block here, making sure - * we've finished it as a sanity check. + * To make these consistent, we always remove the block here. The package + * element count is authoritative, so any bytes after the declared + * elements belong to the following AML terms. */ assert_eq!(context.current_block.kind, BlockKind::Package); - assert_eq!(context.peek(), Err(AmlError::RunOutOfStream)); + let package_end_pc = context.current_block.pc; context.current_block = context.block_stack.pop().unwrap(); + // The package's element count is authoritative. If its encoded length + // extends into following AML terms, resume parsing immediately after the + // declared elements instead of skipping those terms. + context.current_block.pc = package_end_pc; context.contribute_arg(Argument::Object(Object::Package(elements).wrap())); context.retire_op(op); } diff --git a/tests/incorrect_package_length.rs b/tests/incorrect_package_length.rs new file mode 100644 index 00000000..578f653b --- /dev/null +++ b/tests/incorrect_package_length.rs @@ -0,0 +1,17 @@ +mod test_infra; + +use crate::test_infra::run_opcodes_test; +use aml_test_tools::handlers::null_handler::NullHandler; + +#[test] +fn package_with_overlong_length_does_not_consume_following_term() { + // Name (AAAA, Package (2) { 0xA1, 0xA2 }) but the package length is four bytes too long. + // The next term is Name (BBBB, Package (2) { 0xB1, 0xB2 }). + let opcodes = [ + 0x08, b'A', b'A', b'A', b'A', 0x12, 0x0A, 0x02, 0x0A, 0xA1, 0x0A, 0xA2, 0x08, b'B', b'B', b'B', b'B', + 0x12, 0x06, 0x02, 0x0A, 0xB1, 0x0A, 0xB2, 0xA4, 0x92, 0x93, 0x87, b'B', b'B', b'B', b'B', 0x0A, 0x02, + ]; + + // Return zero only if the following Name term was parsed and contains two elements. + run_opcodes_test(&opcodes, NullHandler); +} From c81af3d3608a92b1f51b1bb9a30b59d0e1041173 Mon Sep 17 00:00:00 2001 From: mac Date: Fri, 2 Oct 2026 11:24:42 -0700 Subject: [PATCH 2/2] fix(aml): recover overlong package lengths --- src/aml/mod.rs | 51 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 591f8b72..88bfebf1 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -777,17 +777,34 @@ where * logic to add some `Uninitialized`s, then go round again to complete * the in-flight operation. * - * To make these consistent, we always remove the block here. The package - * element count is authoritative, so any bytes after the declared - * elements belong to the following AML terms. + * To make these consistent, we always remove the block here. If the next + * byte cannot start a package element, it unambiguously starts a following + * AML term even if the package's encoded length extends past it. Bytes + * which could start a package element remain governed by the encoded + * length because their ownership is ambiguous. */ assert_eq!(context.current_block.kind, BlockKind::Package); + let package_suffix = context.peek(); + let suffix_is_package_element = package_suffix.as_ref().is_ok_and(|opcode| { + let extended_opcode = if *opcode == 0x5b { + context.current_block.stream.get(context.current_block.pc + 1).copied() + } else { + None + }; + could_start_package_element(*opcode, extended_opcode) + }); + let package_length_overruns_elements = + package_suffix.is_ok() && !suffix_is_package_element; + assert!( + package_length_overruns_elements || package_suffix == Err(AmlError::RunOutOfStream) + ); let package_end_pc = context.current_block.pc; context.current_block = context.block_stack.pop().unwrap(); - // The package's element count is authoritative. If its encoded length - // extends into following AML terms, resume parsing immediately after the - // declared elements instead of skipping those terms. - context.current_block.pc = package_end_pc; + if package_length_overruns_elements { + // Recover only when the next term cannot be parsed as a package + // element; a NameString may also be an initializer. + context.current_block.pc = package_end_pc; + } context.contribute_arg(Argument::Object(Object::Package(elements).wrap())); context.retire_op(op); } @@ -3372,6 +3389,26 @@ impl MethodContext { } } +/// Whether the first byte(s) can encode a `PackageElement` (DataRefObject or NameString). +fn could_start_package_element(opcode: u8, extended_opcode: Option) -> bool { + match opcode { + 0x00..=0x01 + | 0x0a..=0x0e + | 0x11..=0x13 + | 0x2e..=0x2f + | 0x30..=0x39 + | 0x41..=0x5a + | 0x5c + | 0x5e..=0x6e + | 0x71 + | 0x83 + | 0x88 + | 0xff => true, + 0x5b => matches!(extended_opcode, Some(0x30 | 0x31)), + _ => false, + } +} + #[derive(Clone, Copy, PartialEq, Debug)] enum Opcode { Zero,