[lightbeam] panicked at index out of bound in func_type_index (module.rs) #722

pventuzelo · 2019-12-16T13:07:59Z

Issue description

When calling the translate function of lightbeam, an index out of bound error occurs in function func_type_index because argument func_idx is not checked.

wasmtime/crates/lightbeam/src/module.rs

Lines 395 to 397 in 77bf768

    
           fn func_type_index(&self, func_idx: u32) -> u32 { 
        
               self.func_ty_indicies[func_idx as usize] 
        
           }

This function is called when handling WasmOperator::Call:

wasmtime/crates/lightbeam/src/microwasm.rs

Lines 1165 to 1168 in 77bf768

    
           WasmOperator::Call { function_index } => { 
        
               let func_type = self.module.func_type(*function_index); 
        
               func_type.into() 
        
           }

Fix proposal

2 potentials solutions:

Check if func_index is in the range inside func_type_index function otherwise return an Error.
Check if func_index is in the range each time before calling func_type_index and func_type functions.

Reproduction

wasmtime commit: 77bf768

Download
index_oob_func_type.zip

Just create a basic testing program calling lightbeam translate:

use std::env;
use std::fs::{File};
use std::io;
use std::io::Read;
use std::path::PathBuf;

use lightbeam;

/// Read the contents of a file
fn read_contents(path: &PathBuf) -> Result<Vec<u8>, io::Error> {
    let mut buffer: Vec<u8> = Vec::new();
    let mut file = File::open(path)?;
    file.read_to_end(&mut buffer)?;
    drop(file);
    Ok(buffer)
}

fn main() {
	let args: Vec<String> = env::args().collect();
	let wasm_path = std::path::PathBuf::from(&args[1]);
	let wasm_binary: Vec<u8> = read_contents(&wasm_path).unwrap();

    let _res_translate = lightbeam::translate(&wasm_binary[..]);
}

RUST_BACKTRACE

thread 'main' panicked at 'index out of bounds: the len is 83 but the index is 84', /rustc/2d1a551e144335e0d60a637d12f410cf65849876/src/libcore/slice/mod.rs:2704:10
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.34/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.34/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:47
   3: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:36
   4: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:200
   5: std::panicking::default_hook
             at src/libstd/panicking.rs:214
   6: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:477
   7: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:384
   8: rust_begin_unwind
             at src/libstd/panicking.rs:311
   9: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
  10: core::panicking::panic_bounds_check
             at src/libcore/panicking.rs:61
  11: <usize as core::slice::SliceIndex<[T]>>::index
             at /rustc/2d1a551e144335e0d60a637d12f410cf65849876/src/libcore/slice/mod.rs:2704
  12: core::slice::<impl core::ops::index::Index<I> for [T]>::index
             at /rustc/2d1a551e144335e0d60a637d12f410cf65849876/src/libcore/slice/mod.rs:2555
  13: <alloc::vec::Vec<T> as core::ops::index::Index<I>>::index
             at /rustc/2d1a551e144335e0d60a637d12f410cf65849876/src/liballoc/vec.rs:1791
  14: <lightbeam::module::SimpleContext as lightbeam::module::ModuleContext>::func_type_index
             at XXX/wasmtime/crates/lightbeam/src/module.rs:396
  15: lightbeam::module::ModuleContext::func_type
             at XXX/wasmtime/crates/lightbeam/src/module.rs:374
  16: lightbeam::microwasm::MicrowasmConv<M>::op_sig
             at XXX/wasmtime/crates/lightbeam/src/microwasm.rs:1166
  17: <lightbeam::microwasm::MicrowasmConv<M> as core::iter::traits::iterator::Iterator>::next
             at XXX/wasmtime/crates/lightbeam/src/microwasm.rs:1604
  18: lightbeam::function_body::translate_wasm
             at XXX/wasmtime/crates/lightbeam/src/function_body.rs:75
  19: lightbeam::translate_sections::code
             at XXX/wasmtime/crates/lightbeam/src/translate_sections.rs:118
  20: lightbeam::module::translate_only
             at XXX/wasmtime/crates/lightbeam/src/module.rs:631
  21: lightbeam::module::translate
             at XXX/wasmtime/crates/lightbeam/src/module.rs:504
  22: debug_lightbeam::main
             at src/main.rs:34
  23: std::rt::lang_start::{{closure}}
             at /rustc/2d1a551e144335e0d60a637d12f410cf65849876/src/libstd/rt.rs:64
  24: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:49
  25: std::panicking::try::do_call
             at src/libstd/panicking.rs:296
  26: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:80
  27: std::panicking::try
             at src/libstd/panicking.rs:275
  28: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  29: std::rt::lang_start_internal
             at src/libstd/rt.rs:48
  30: std::rt::lang_start
             at /rustc/2d1a551e144335e0d60a637d12f410cf65849876/src/libstd/rt.rs:64
  31: main
  32: __libc_start_main
  33: _start
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

The text was updated successfully, but these errors were encountered:

Robbepop · 2019-12-16T13:21:16Z

Shouldn't at this point the fed Wasm already be validated by the parser (probably wasmparser) so that later stages can simply assume these values to be in range? If the input Wasm is not valid we should definitely return a recoverable error from Lightbeam to the caller.

pventuzelo · 2019-12-16T13:24:34Z

This module is valid actually:

$ wasm-validate index_oob_func_type.wasm && echo success
success

Same result for wasmparser:validate

Robbepop · 2019-12-16T13:30:50Z

Ah ok.
Then this to me looks more like an internal compiler bug but not that I actual understand Lightbeam in depth. So if the input Wasm is correct according to the validation, either it is a validation bug or I am missing some opportunities of how else we could have function indices that are out of range.

I am not sure if the proposed solutions would actually "fix" the bug:

2 potentials solutions:
- Check if func_index is in the range inside func_type_index function otherwise return an Error.
- Check if func_index is in the range each time before calling func_type_index and func_type functions.

Because we should normally only return errors for invalid input or input that requires Wasm features we do not yet support. For everything else we probably have a bug that we should either fix or gracefully abort the process and report the issue which we do already by the index-out-of-bounds panic.

Also for the sake of compiler performance we should not check something that we should assume to be true due to given invariances.

pventuzelo · 2019-12-16T13:36:23Z

For me it's a runtime error. For example, this specific case is checked in execute_func i.e. just before calling func_type.

wasmtime/crates/lightbeam/src/module.rs

Lines 185 to 190 in 77bf768

    
           if func_idx as usize >= module.ctx.func_ty_indicies.len() { 
        
               return Err(ExecutionError::FuncIndexOutOfBounds); 
        
           } 
        
           let type_ = module.ctx.func_type(func_idx);

Robbepop · 2019-12-16T14:08:23Z

For me it's a runtime error. For example, this specific case is checked in execute_func i.e. just before calling func_type.

wasmtime/crates/lightbeam/src/module.rs

Lines 185 to 190 in 77bf768

if func_idx as usize >= module.ctx.func_ty_indicies.len() {

return Err(ExecutionError::FuncIndexOutOfBounds);

}

let type_ = module.ctx.func_type(func_idx);

True, but I guess at this point only @Vurich is capable of properly answering this because this goes into the Lightbeam implementation details now.

pepyakin · 2019-12-16T16:40:37Z

function_index in WasmOperator::Call contained in a valid sequence of code must always point to a valid function, so yeah it seems that there is another bug somewhere here.

eira-fransham · 2019-12-17T16:49:09Z

This feels like something that should be trivial to find a reduced testcase for, which might be more enlightening about it. Right now to debug this we'd have to basically just look at the code and work out what could be the issue from first principles, and we won't be sure that this testcase passing would actually mean the bug is fixed. We could do that but I feel like making a reduced testcase for this should be easy, I can't see it being a complex bug.

pventuzelo · 2019-12-17T19:44:48Z

Here is a minimized testcase:

(module
  (type (;0;) (func))
  (type (;1;) (func (param i32) (result i32)))
  (import "env" "b" (func (;0;) (type 0)))
  (import "env" "c" (func (;1;) (type 0)))
  (import "env" "d" (func (;2;) (type 0)))
  (import "env" "e" (func (;3;) (type 0)))
  (import "env" "f" (func (;4;) (type 0)))
  (import "env" "g" (func (;5;) (type 0)))
  (import "env" "h" (func (;6;) (type 0)))
  (import "env" "i" (func (;7;) (type 0)))
  (import "env" "j" (func (;8;) (type 0)))
  (func (;9;) (type 1) (param i32) (result i32)
    call 10
    i32.const 9)
  (func (;10;) (type 0)
    unreachable)
  (export "_start" (func 9))
)

After execution, we get the following error:

thread 'main' panicked at 'index out of bounds: the len is 2 but the index is 10', /rustc/2d1a551e144335e0d60a637d12f410cf65849876/src/libcore/slice/mod.rs:2704:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

So, i suppose that the issue is because imported functions are not added to the list all callable functions.

Robbepop · 2019-12-17T22:24:49Z

Are imports generally already processed at all?
From what I found so far it doesn't look like it:
https://github.com/bytecodealliance/wasmtime/blob/master/crates/lightbeam/src/translate_sections.rs#L21
Looks the same for global, export, element and start_fn sections.

pventuzelo · 2019-12-18T08:44:03Z

Yes exactly.

eira-fransham · 2020-01-07T09:57:53Z

Are imports generally already processed at all?
From what I found so far it doesn't look like it:
https://github.com/bytecodealliance/wasmtime/blob/master/crates/lightbeam/src/translate_sections.rs#L21
Looks the same for global, export, element and start_fn sections.

This is for the local test running harness, which is essentially cruft that I haven't removed yet because it would require porting the Lightbeam test cases into Wasmtime test cases. These (i.e. globals, exports etc) should be handled by Wasmtime, which is why you don't see that code there. The original bug does appear to be in Lightbeam, however.

alexcrichton · 2021-09-27T17:29:28Z

Lightbeam was removed in #3390 as explained in RFC 14, so I'm going to close this.

pepyakin added lightbeam Issues related to the Lightbeam compiler fuzz-bug Bugs found by a fuzzer labels Jan 6, 2020

alexcrichton closed this as completed Sep 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lightbeam] panicked at index out of bound in func_type_index (module.rs) #722

[lightbeam] panicked at index out of bound in func_type_index (module.rs) #722

pventuzelo commented Dec 16, 2019

Robbepop commented Dec 16, 2019 •

edited

Loading

pventuzelo commented Dec 16, 2019

Robbepop commented Dec 16, 2019 •

edited

Loading

pventuzelo commented Dec 16, 2019

Robbepop commented Dec 16, 2019 •

edited

Loading

pepyakin commented Dec 16, 2019 •

edited

Loading

eira-fransham commented Dec 17, 2019

pventuzelo commented Dec 17, 2019

Robbepop commented Dec 17, 2019 •

edited

Loading

pventuzelo commented Dec 18, 2019

eira-fransham commented Jan 7, 2020 •

edited

Loading

alexcrichton commented Sep 27, 2021

[lightbeam] panicked at index out of bound in func_type_index (module.rs) #722

[lightbeam] panicked at index out of bound in func_type_index (module.rs) #722

Comments

pventuzelo commented Dec 16, 2019

Issue description

Fix proposal

Reproduction

RUST_BACKTRACE

Robbepop commented Dec 16, 2019 • edited Loading

pventuzelo commented Dec 16, 2019

Robbepop commented Dec 16, 2019 • edited Loading

pventuzelo commented Dec 16, 2019

Robbepop commented Dec 16, 2019 • edited Loading

pepyakin commented Dec 16, 2019 • edited Loading

eira-fransham commented Dec 17, 2019

pventuzelo commented Dec 17, 2019

Robbepop commented Dec 17, 2019 • edited Loading

pventuzelo commented Dec 18, 2019

eira-fransham commented Jan 7, 2020 • edited Loading

alexcrichton commented Sep 27, 2021

Robbepop commented Dec 16, 2019 •

edited

Loading

Robbepop commented Dec 16, 2019 •

edited

Loading

Robbepop commented Dec 16, 2019 •

edited

Loading

pepyakin commented Dec 16, 2019 •

edited

Loading

Robbepop commented Dec 17, 2019 •

edited

Loading

eira-fransham commented Jan 7, 2020 •

edited

Loading