From 04e6d5d5538cc8514b3e82d147586d56d34e5a9a Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Wed, 10 Apr 2024 14:19:46 -0400 Subject: [PATCH] Fix panic when loading malformed Wasm files (cherry-pick #10370) (#10371) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-picked Fix panic when loading malformed Wasm files (#10370) This PR fixes a potential panic that could occur when loading malformed Wasm files. We now use the `parse_wasm_extension_version` function that was previously used just to extract the Zed extension API version from the Wasm bytes as a pre-validation step. By parsing the entirety of the Wasm file here instead of returning as soon as we find the version, the invalid Wasm bytes are now surfaced as an `Err` instead of a panic. We were able to replicate the panic using the following test: ```rs #[gpui::test] async fn test_bad_wasm(cx: &mut TestAppContext) { init_test(cx); let wasm_host = cx.update(|cx| { WasmHost::new( FakeFs::new(cx.background_executor().clone()), FakeHttpClient::with_200_response(), FakeNodeRuntime::new(), Arc::new(LanguageRegistry::test(cx.background_executor().clone())), PathBuf::from("/the/work/dir".to_string()), cx, ) }); let mut wasm_bytes = std::fs::read("/Users/maxdeviant/Library/Application Support/Zed/extensions/installed/dart/extension.wasm").unwrap(); // This is the error message we were seeing in the stack trace: // range end index 267037 out of range for slice of length 253952 dbg!(&wasm_bytes.len()); // Truncate the bytes to the same point: wasm_bytes.truncate(253952); std::fs::write("/tmp/bad-extension.wasm", wasm_bytes.clone()).unwrap(); let manifest = Arc::new(ExtensionManifest { id: "the-extension".into(), name: "The Extension".into(), version: "0.0.1".into(), schema_version: SchemaVersion(1), description: Default::default(), repository: Default::default(), authors: Default::default(), lib: LibManifestEntry { kind: None, version: None, }, themes: Default::default(), languages: Default::default(), grammars: Default::default(), language_servers: Default::default(), }); // 💥 let result = wasm_host .load_extension(wasm_bytes, manifest, cx.executor()) .await; dbg!(result.map(|_| ())); ``` Release Notes: - Fixed a crash that could occur when loading malformed Wasm extensions ([#10352](https://github.com/zed-industries/zed/issues/10352)). --------- Co-authored-by: Max Co-authored-by: Marshall Bowers Co-authored-by: Max --- crates/extension/src/wasm_host.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/extension/src/wasm_host.rs b/crates/extension/src/wasm_host.rs index e4c97d2213244c386ef9ee60028ed76ea65e922f..51b306f1ff3c3b13752f44bdd550670e65581970 100644 --- a/crates/extension/src/wasm_host.rs +++ b/crates/extension/src/wasm_host.rs @@ -198,13 +198,15 @@ pub fn parse_wasm_extension_version( extension_id: &str, wasm_bytes: &[u8], ) -> Result { + let mut version = None; + for part in wasmparser::Parser::new(0).parse_all(wasm_bytes) { - if let wasmparser::Payload::CustomSection(s) = part? { + if let wasmparser::Payload::CustomSection(s) = + part.context("error parsing wasm extension")? + { if s.name() == "zed:api-version" { - let version = parse_wasm_extension_version_custom_section(s.data()); - if let Some(version) = version { - return Ok(version); - } else { + version = parse_wasm_extension_version_custom_section(s.data()); + if version.is_none() { bail!( "extension {} has invalid zed:api-version section: {:?}", extension_id, @@ -214,7 +216,13 @@ pub fn parse_wasm_extension_version( } } } - bail!("extension {} has no zed:api-version section", extension_id) + + // The reason we wait until we're done parsing all of the Wasm bytes to return the version + // is to work around a panic that can happen inside of Wasmtime when the bytes are invalid. + // + // By parsing the entirety of the Wasm bytes before we return, we're able to detect this problem + // earlier as an `Err` rather than as a panic. + version.ok_or_else(|| anyhow!("extension {} has no zed:api-version section", extension_id)) } fn parse_wasm_extension_version_custom_section(data: &[u8]) -> Option {