Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic in table.init #8281

Closed
ShinWonho opened this issue Apr 2, 2024 · 3 comments · Fixed by #8283
Closed

Panic in table.init #8281

ShinWonho opened this issue Apr 2, 2024 · 3 comments · Fixed by #8283
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@ShinWonho
Copy link

ShinWonho commented Apr 2, 2024

Test Case

;; table-init.wat
(module
  (table 0 0 externref)
  (func (export "table-init")
    (i32.const 0)
    (i32.const 0)
    (i32.const 0)
    (table.init 0 0)
  )
  (elem declare externref)
)

Steps to Reproduce

wasmtime --invoke table-init table-init.wat

Expected Results

terminate normally

Actual Results

thread 'main' panicked at crates/runtime/src/table.rs:539:9:
assertion `left == right` failed
  left: Extern
 right: Func
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
zsh: abort      wasmtime --invoke table-init table-init.wat

Versions and Environment

Wasmtime version or commit: 19.0.0

Operating system: macOS Ventura 13.6.6

Architecture: x86_64

@ShinWonho ShinWonho added the bug Incorrect behavior in the current implementation that needs fixing label Apr 2, 2024
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Apr 2, 2024
This commit fixes an accidental issue introduced in bytecodealliance#8018 where using an
element segment which had been dropped with an `externref` table would
cause a panic. The panic happened due to an assertion that tables are
being used with the right type of item and that was being mismatched.
The underlying issue was that dropped element segments are modeled as an
empty element segment but the empty element segment was using the
"functions" encoding as opposed to the "expressions" encoding. This
meant that code later assumed that due to the use of functions the table
must be a table-of-functions, but this was not correct for
externref-based tables.

The fix in this commit is to instead model the encoding as an
"expressions" list which means that the table type is dispatched on to
call the appropriate initializer.

There is no memory safety issue with this mistake as the assertion was
specifically targetted at preventing memory safety. This does, however,
enable any WebAssembly module to panic a host.

Closes bytecodealliance#8281
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Apr 2, 2024
This commit fixes an accidental issue introduced in bytecodealliance#8018 where using an
element segment which had been dropped with an `externref` table would
cause a panic. The panic happened due to an assertion that tables are
being used with the right type of item and that was being mismatched.
The underlying issue was that dropped element segments are modeled as an
empty element segment but the empty element segment was using the
"functions" encoding as opposed to the "expressions" encoding. This
meant that code later assumed that due to the use of functions the table
must be a table-of-functions, but this was not correct for
externref-based tables.

The fix in this commit is to instead model the encoding as an
"expressions" list which means that the table type is dispatched on to
call the appropriate initializer.

There is no memory safety issue with this mistake as the assertion was
specifically targetted at preventing memory safety. This does, however,
enable any WebAssembly module to panic a host.

Closes bytecodealliance#8281
@alexcrichton
Copy link
Member

alexcrichton commented Apr 2, 2024

Thanks for the report! Do you perhaps have more detail on how this was discovered? For example is this a reduced module? Or perhaps a fuzz-generated test case?

Also as per our documentation this is a security issue so we'll be issuing a CVE and a 19.0.1 release for this. If you discover more issues like this we'd be grateful if you'd contact us privately so we can coordinate this, thanks!

alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Apr 2, 2024
This commit fixes an accidental issue introduced in bytecodealliance#8018 where using an
element segment which had been dropped with an `externref` table would
cause a panic. The panic happened due to an assertion that tables are
being used with the right type of item and that was being mismatched.
The underlying issue was that dropped element segments are modeled as an
empty element segment but the empty element segment was using the
"functions" encoding as opposed to the "expressions" encoding. This
meant that code later assumed that due to the use of functions the table
must be a table-of-functions, but this was not correct for
externref-based tables.

The fix in this commit is to instead model the encoding as an
"expressions" list which means that the table type is dispatched on to
call the appropriate initializer.

There is no memory safety issue with this mistake as the assertion was
specifically targetted at preventing memory safety. This does, however,
enable any WebAssembly module to panic a host.

Closes bytecodealliance#8281
github-merge-queue bot pushed a commit that referenced this issue Apr 2, 2024
This commit fixes an accidental issue introduced in #8018 where using an
element segment which had been dropped with an `externref` table would
cause a panic. The panic happened due to an assertion that tables are
being used with the right type of item and that was being mismatched.
The underlying issue was that dropped element segments are modeled as an
empty element segment but the empty element segment was using the
"functions" encoding as opposed to the "expressions" encoding. This
meant that code later assumed that due to the use of functions the table
must be a table-of-functions, but this was not correct for
externref-based tables.

The fix in this commit is to instead model the encoding as an
"expressions" list which means that the table type is dispatched on to
call the appropriate initializer.

There is no memory safety issue with this mistake as the assertion was
specifically targetted at preventing memory safety. This does, however,
enable any WebAssembly module to panic a host.

Closes #8281
alexcrichton added a commit that referenced this issue Apr 2, 2024
This commit fixes an accidental issue introduced in #8018 where using an
element segment which had been dropped with an `externref` table would
cause a panic. The panic happened due to an assertion that tables are
being used with the right type of item and that was being mismatched.
The underlying issue was that dropped element segments are modeled as an
empty element segment but the empty element segment was using the
"functions" encoding as opposed to the "expressions" encoding. This
meant that code later assumed that due to the use of functions the table
must be a table-of-functions, but this was not correct for
externref-based tables.

The fix in this commit is to instead model the encoding as an
"expressions" list which means that the table type is dispatched on to
call the appropriate initializer.

There is no memory safety issue with this mistake as the assertion was
specifically targetted at preventing memory safety. This does, however,
enable any WebAssembly module to panic a host.

Closes #8281
@ShinWonho
Copy link
Author

ShinWonho commented Apr 3, 2024

We are in progress of implementing a wasm fuzzer based on SpecTec. Currently, it generates short wasm programs by a simple syntax-driven approach. We performed differential testing with the latest wasmtime and previous versions, and luckily found the bug. We reduced the buggy program manually as the generated program was simple.

@alexcrichton
Copy link
Member

Nice! If y'all need any help with fuzzing or such we're happy to talk as well. And thank you for fuzzing, we very much appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants