diff --git a/CHANGELOG.md b/CHANGELOG.md index f32e796956..4e3f67126c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Fixed debug-only underflow in memory range-check trace generation when the first memory access is at `clk = 0` ([#2976](https://github.com/0xMiden/miden-vm/pull/2976)). - Replaced unsound `ptr::read` with safe unbox in panic recovery, removing UB from potential double-drop ([#2934](https://github.com/0xMiden/miden-vm/pull/2934)). +- Library deserialization now rejects exports whose `MastNodeId` is not a procedure root, closing a silent-failure path ([#2933](https://github.com/0xMiden/miden-vm/pull/2933)). - Reverted `InvokeKind::ProcRef` back to `InvokeKind::Exec` in `visit_mut_procref` and added an explanatory comment (#2893). - Fixed the release dry-run publish cycle between `miden-air` and `miden-ace-codegen`, and preserved leaf-only DAG imports with explicit snapshots ([#2931](https://github.com/0xMiden/miden-vm/pull/2931)). - Fixed AEAD padding handling so encrypt does not overwrite memory next to the plaintext buffer and decrypt leaves the plaintext output tail untouched ([#3008](https://github.com/0xMiden/miden-vm/pull/3008)). diff --git a/crates/assembly-syntax/src/library/mod.rs b/crates/assembly-syntax/src/library/mod.rs index 53afe6d831..1d1655e17e 100644 --- a/crates/assembly-syntax/src/library/mod.rs +++ b/crates/assembly-syntax/src/library/mod.rs @@ -648,13 +648,8 @@ impl Deserializable for Library { exports.insert(path, export); } - let digest = - mast_forest.compute_nodes_commitment(exports.values().filter_map(|e| match e { - LibraryExport::Procedure(e) => Some(&e.node), - LibraryExport::Constant(_) | LibraryExport::Type(_) => None, - })); - - Ok(Self { digest, exports, mast_forest }) + Self::new(mast_forest, exports) + .map_err(|err| DeserializationError::InvalidValue(format!("{err}"))) } } diff --git a/crates/assembly/src/tests.rs b/crates/assembly/src/tests.rs index ce50ae4d0c..f36a5a3b56 100644 --- a/crates/assembly/src/tests.rs +++ b/crates/assembly/src/tests.rs @@ -411,6 +411,66 @@ fn library_serialization() -> Result<(), Report> { Ok(()) } +/// Verifies that deserializing a library rejects procedure exports whose `MastNodeId` is not a +/// procedure root in the underlying MAST forest (issue #2831). +#[test] +fn library_deserialization_rejects_non_root_export() { + use miden_core::{ + mast::{BasicBlockNodeBuilder, MastForestContributor}, + serde::ByteWriter, + }; + + let context = TestContext::new(); + let source = r#" + pub proc foo + add + end + "#; + let module = parse_module!(&context, "test::foo", source); + + // Build a valid library. + let lib = Assembler::new(context.source_manager()).assemble_library([module]).unwrap(); + + // Clone the forest and add a non-root node. + let mut forest: MastForest = (**lib.mast_forest()).clone(); + let builder = BasicBlockNodeBuilder::new(vec![Operation::Add], vec![]); + let non_root_id = builder.add_to_forest(&mut forest).unwrap(); + assert!( + !forest.is_procedure_root(non_root_id), + "sanity check: new node should not be a root" + ); + + // Manually serialize a tampered library: forest + one export referencing the non-root node. + let mut tampered_bytes = Vec::new(); + forest.write_into(&mut tampered_bytes); + + // Number of exports. + 1usize.write_into(&mut tampered_bytes); + // Tag: 0 = Procedure export. + 0u8.write_into(&mut tampered_bytes); + // Fully qualified procedure path. + let path = PathBuf::new("::test::foo::foo").unwrap(); + path.write_into(&mut tampered_bytes); + // MastNodeId of the non-root node. + u32::from(non_root_id).write_into(&mut tampered_bytes); + // No function signature. + tampered_bytes.write_bool(false); + // Empty attribute set. + miden_assembly_syntax::ast::AttributeSet::default().write_into(&mut tampered_bytes); + + // Deserializing should fail because the export references a non-root node. + let result = Library::read_from_bytes(&tampered_bytes); + assert!( + result.is_err(), + "deserialization should reject exports referencing non-root nodes" + ); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("no procedure root"), + "error should mention missing procedure root, got: {err_msg}" + ); +} + #[test] fn get_module_by_path() -> Result<(), Report> { let context = TestContext::new();