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 <max@zed.dev>
This commit is contained in:
@@ -198,13 +198,15 @@ pub fn parse_wasm_extension_version(
|
||||
extension_id: &str,
|
||||
wasm_bytes: &[u8],
|
||||
) -> Result<SemanticVersion> {
|
||||
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<SemanticVersion> {
|
||||
|
||||
Reference in New Issue
Block a user