Call-indirect caching: protect against out-of-bounds table index during prescan. (bytecodealliance#8541)

* Call-indirect caching: protect against out-of-bounds table index during prescan.

Call-indirect caching requires a "prescan" of a module's code section
during compilation in order to know which tables are possibly written,
and to count call-indirect callsites so that each separate function
compilation can enable caching if possible and use the right slot
range.

This prescan is not integrated with the usual validation logic (nor
should it be, probably, for performance), so it's possible that an
out-of-bounds table index or other illegal instruction could be
present. We previously indexed into an internal data
structure (`table_plans`) with this index, allowing for a compilation
panic on certain invalid modules before validation would have caught
it. This PR fixes that with a one-off check at the single point that
we interpret a parameter (the table index) from an instruction during
the prescan.

* Add test case.

Author: cfallin

crates/environ/src/compile/module_environ.rs

@@ -11,6 +11,7 @@ use crate::{
 };
 use anyhow::{bail, Result};
 use cranelift_entity::packed_option::ReservedValue;
+use cranelift_entity::EntityRef;
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::mem;
@@ -737,7 +738,14 @@ and for re-adding support for interface types you can see this issue:
                     | Operator::TableCopy {
                         dst_table: table, ..
                     } => {
-                        self.flag_written_table(TableIndex::from_u32(table));
+                        // We haven't yet validated the body during
+                        // this pre-scan, so we need to check that
+                        // `dst_table` is in bounds. Ignore if not:
+                        // we'll catch the error later.
+                        let table = TableIndex::from_u32(table);
+                        if table.index() < self.result.module.table_plans.len() {
+                            self.flag_written_table(table);
+                        }
                     }
                     // Count the `call_indirect` sites so we can
                     // assign them unique slots.

tests/all/module.rs

@@ -269,3 +269,31 @@ fn call_indirect_caching_and_memory64() -> Result<()> {
     )?;
     Ok(())
 }
+
+#[test]
+fn call_indirect_caching_out_of_bounds_table_index() -> Result<()> {
+    let mut config = Config::new();
+    config.cache_call_indirects(true);
+    let engine = Engine::new(&config)?;
+    // Test an out-of-bounds table index: this is exposed to the prescan
+    // that call-indirect caching must perform during compilation, so we
+    // need to make sure the error is properly handled by the validation
+    // that comes later.
+    let err = Module::new(
+        &engine,
+        "(module
+            (func (param i32)
+                ref.null func
+                local.get 0
+                table.set 32  ;; out-of-bounds table index
+            )
+        )",
+    )
+    .unwrap_err();
+    let err = format!("{err:?}");
+    assert!(
+        err.contains("table index out of bounds"),
+        "bad error: {err}"
+    );
+    Ok(())
+}