From 9971d13844619caa772fd772f63c83ace8cef750 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Mon, 21 Jun 2021 21:02:17 +0430 Subject: [PATCH] LibWasm: Trap if a non-Value is used as a Value Otherwise we'd just crash, which is not a good thing --- .../AbstractMachine/BytecodeInterpreter.cpp | 84 ++++++++++++++----- 1 file changed, 61 insertions(+), 23 deletions(-) diff --git a/Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp b/Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp index 1087a99bdb..6a9b620ea8 100644 --- a/Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp +++ b/Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp @@ -82,7 +82,9 @@ void BytecodeInterpreter::load_and_push(Configuration& configuration, Instructio return; } auto& arg = instruction.arguments().get(); - auto base = configuration.stack().peek().get().to(); + auto& entry = configuration.stack().peek(); + TRAP_IF_NOT(entry.has()); + auto base = entry.get().to(); if (!base.has_value()) { m_do_trap = true; return; @@ -104,7 +106,9 @@ void BytecodeInterpreter::store_to_memory(Configuration& configuration, Instruct auto memory = configuration.store().get(address); TRAP_IF_NOT(memory); auto& arg = instruction.arguments().get(); - auto base = configuration.stack().pop().get().to(); + auto entry = configuration.stack().pop(); + TRAP_IF_NOT(entry.has()); + auto base = entry.get().to(); TRAP_IF_NOT(base.has_value()); auto instance_address = base.value() + static_cast(arg.offset); if (instance_address < 0 || static_cast(instance_address + data.size()) > memory->size()) { @@ -153,8 +157,12 @@ void BytecodeInterpreter::call_address(Configuration& configuration, FunctionAdd #define BINARY_NUMERIC_OPERATION(type, operator, cast, ...) \ do { \ - auto rhs = configuration.stack().pop().get().to(); \ - auto lhs = configuration.stack().peek().get().to(); \ + auto rhs_entry = configuration.stack().pop(); \ + auto& lhs_entry = configuration.stack().peek(); \ + TRAP_IF_NOT(rhs_entry.has()); \ + TRAP_IF_NOT(lhs_entry.has()); \ + auto rhs = rhs_entry.get().to(); \ + auto lhs = lhs_entry.get().to(); \ TRAP_IF_NOT(lhs.has_value()); \ TRAP_IF_NOT(rhs.has_value()); \ __VA_ARGS__; \ @@ -166,8 +174,12 @@ void BytecodeInterpreter::call_address(Configuration& configuration, FunctionAdd #define OVF_CHECKED_BINARY_NUMERIC_OPERATION(type, operator, cast, ...) \ do { \ - auto rhs = configuration.stack().pop().get().to(); \ - auto ulhs = configuration.stack().peek().get().to(); \ + auto rhs_entry = configuration.stack().pop(); \ + auto& lhs_entry = configuration.stack().peek(); \ + TRAP_IF_NOT(rhs_entry.has()); \ + TRAP_IF_NOT(lhs_entry.has()); \ + auto rhs = rhs_entry.get().to(); \ + auto ulhs = lhs_entry.get().to(); \ TRAP_IF_NOT(ulhs.has_value()); \ TRAP_IF_NOT(rhs.has_value()); \ dbgln_if(WASM_TRACE_DEBUG, "{} {} {} = ??", ulhs.value(), #operator, rhs.value()); \ @@ -183,8 +195,12 @@ void BytecodeInterpreter::call_address(Configuration& configuration, FunctionAdd #define BINARY_PREFIX_NUMERIC_OPERATION(type, operation, cast, ...) \ do { \ - auto rhs = configuration.stack().pop().get().to(); \ - auto lhs = configuration.stack().peek().get().to(); \ + auto rhs_entry = configuration.stack().pop(); \ + auto& lhs_entry = configuration.stack().peek(); \ + TRAP_IF_NOT(rhs_entry.has()); \ + TRAP_IF_NOT(lhs_entry.has()); \ + auto rhs = rhs_entry.get().to(); \ + auto lhs = lhs_entry.get().to(); \ TRAP_IF_NOT(lhs.has_value()); \ TRAP_IF_NOT(rhs.has_value()); \ auto result = operation(lhs.value(), rhs.value()); \ @@ -195,7 +211,9 @@ void BytecodeInterpreter::call_address(Configuration& configuration, FunctionAdd #define UNARY_MAP(pop_type, operation, ...) \ do { \ - auto value = configuration.stack().peek().get().to(); \ + auto& entry = configuration.stack().peek(); \ + TRAP_IF_NOT(entry.has()); \ + auto value = entry.get().to(); \ TRAP_IF_NOT(value.has_value()); \ auto result = operation(value.value()); \ dbgln_if(WASM_TRACE_DEBUG, "map({}) {} = {}", #operation, value.value(), result); \ @@ -211,12 +229,14 @@ void BytecodeInterpreter::call_address(Configuration& configuration, FunctionAdd return load_and_push(configuration, instruction); \ } while (false) -#define POP_AND_STORE(pop_type, store_type) \ - do { \ - auto value = ConvertToRaw {}(*configuration.stack().pop().get().to()); \ - dbgln_if(WASM_TRACE_DEBUG, "stack({}) -> temporary({}b)", value, sizeof(store_type)); \ - store_to_memory(configuration, instruction, { &value, sizeof(store_type) }); \ - return; \ +#define POP_AND_STORE(pop_type, store_type) \ + do { \ + auto entry = configuration.stack().pop(); \ + TRAP_IF_NOT(entry.has()); \ + auto value = ConvertToRaw {}(*entry.get().to()); \ + dbgln_if(WASM_TRACE_DEBUG, "stack({}) -> temporary({}b)", value, sizeof(store_type)); \ + store_to_memory(configuration, instruction, { &value, sizeof(store_type) }); \ + return; \ } while (false) template @@ -472,6 +492,7 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi return; case Instructions::local_set.value(): { auto entry = configuration.stack().pop(); + TRAP_IF_NOT(entry.has()); configuration.frame().locals()[instruction.arguments().get().value()] = move(entry.get()); return; } @@ -510,6 +531,7 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi arity = 1; auto entry = configuration.stack().pop(); + TRAP_IF_NOT(entry.has()); auto value = entry.get().to(); TRAP_IF_NOT(value.has_value()); auto end_label = Label(arity, args.end_ip.value()); @@ -565,7 +587,9 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi case Instructions::br.value(): return branch_to_label(configuration, instruction.arguments().get()); case Instructions::br_if.value(): { - if (configuration.stack().pop().get().to().value_or(0) == 0) + auto entry = configuration.stack().pop(); + TRAP_IF_NOT(entry.has()); + if (entry.get().to().value_or(0) == 0) return; return branch_to_label(configuration, instruction.arguments().get()); } @@ -594,7 +618,9 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi TRAP_IF_NOT(args.table.value() < configuration.frame().module().tables().size()); auto table_address = configuration.frame().module().tables()[args.table.value()]; auto table_instance = configuration.store().get(table_address); - auto index = configuration.stack().pop().get().to(); + auto entry = configuration.stack().pop(); + TRAP_IF_NOT(entry.has()); + auto index = entry.get().to(); TRAP_IF_NOT(index.has_value()); TRAP_IF_NOT(index.value() >= 0); TRAP_IF_NOT(static_cast(index.value()) < table_instance->elements().size()); @@ -653,7 +679,9 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi case Instructions::i64_store32.value(): POP_AND_STORE(i64, i32); case Instructions::local_tee.value(): { - auto value = configuration.stack().peek().get(); + auto& entry = configuration.stack().peek(); + TRAP_IF_NOT(entry.has()); + auto value = entry.get(); auto local_index = instruction.arguments().get(); TRAP_IF_NOT(configuration.frame().locals().size() > local_index.value()); dbgln_if(WASM_TRACE_DEBUG, "stack:peek -> locals({})", local_index.value()); @@ -673,7 +701,9 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi auto global_index = instruction.arguments().get(); TRAP_IF_NOT(configuration.frame().module().globals().size() > global_index.value()); auto address = configuration.frame().module().globals()[global_index.value()]; - auto value = configuration.stack().pop().get(); + auto entry = configuration.stack().pop(); + TRAP_IF_NOT(entry.has()); + auto value = entry.get(); dbgln_if(WASM_TRACE_DEBUG, "stack -> global({})", address.value()); auto global = configuration.store().get(address); global->set_value(move(value)); @@ -693,7 +723,9 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi auto address = configuration.frame().module().memories()[0]; auto instance = configuration.store().get(address); i32 old_pages = instance->size() / Constants::page_size; - auto new_pages = configuration.stack().peek().get().to(); + auto& entry = configuration.stack().peek(); + TRAP_IF_NOT(entry.has()); + auto new_pages = entry.get().to(); TRAP_IF_NOT(new_pages.has_value()); dbgln_if(WASM_TRACE_DEBUG, "memory.grow({}), previously {} pages...", *new_pages, old_pages); if (instance->grow(new_pages.value() * Constants::page_size)) @@ -733,11 +765,17 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi case Instructions::select.value(): case Instructions::select_typed.value(): { // Note: The type seems to only be used for validation. - auto value = configuration.stack().pop().get().to(); + auto entry = configuration.stack().pop(); + TRAP_IF_NOT(entry.has()); + auto value = entry.get().to(); TRAP_IF_NOT(value.has_value()); dbgln_if(WASM_TRACE_DEBUG, "select({})", value.value()); - auto rhs = move(configuration.stack().pop().get()); - auto lhs = move(configuration.stack().peek().get()); + auto rhs_entry = configuration.stack().pop(); + TRAP_IF_NOT(rhs_entry.has()); + auto& lhs_entry = configuration.stack().peek(); + TRAP_IF_NOT(lhs_entry.has()); + auto rhs = move(rhs_entry.get()); + auto lhs = move(lhs_entry.get()); configuration.stack().peek() = value.value() != 0 ? move(lhs) : move(rhs); return; }