From bdc0b31ea0792ef210fff178785b6b6d3f0320b4 Mon Sep 17 00:00:00 2001 From: Marek Marecki Date: Tue, 23 May 2023 22:04:21 +0200 Subject: [PATCH] Fix implementations of SM and LM instructions The addresses need to be checked by the VM to prevent badly written, or just malicious, programs from trashing VM's memory. --- new/include/viua/vm/core.h | 7 ++++++ new/src/vm/ins.cpp | 46 +++++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/new/include/viua/vm/core.h b/new/include/viua/vm/core.h index 1d54342ed..b35120db0 100644 --- a/new/include/viua/vm/core.h +++ b/new/include/viua/vm/core.h @@ -445,6 +445,13 @@ struct Process { using Pointer = std::pair; auto memory_at(Pointer const) const -> void const*; auto memory_at(Pointer const) -> void*; + inline auto memory_at(size_t const ptr) -> uint8_t* + { + if (ptr >= (MEM_PAGE_SIZE * memory.size())) { + return nullptr; + } + return memory.front().data() + ptr; + } explicit inline Process(pid_type const p, Core* c, Module const& m) : pid{p}, core{c}, module{m}, strtab{&m.strings_table}, stack{*this} diff --git a/new/src/vm/ins.cpp b/new/src/vm/ins.cpp index f2c4e77af..7a12c3f14 100644 --- a/new/src/vm/ins.cpp +++ b/new/src/vm/ins.cpp @@ -2078,36 +2078,41 @@ auto execute(ECALL const, Stack&, ip_type const) -> void auto execute(SM const op, Stack& stack, ip_type const ip) -> void { - auto& registers = stack.frames.back().registers; - auto const base = get_value(registers, op.instruction.in, ip); + auto const base = get_value(stack, op.instruction.in, ip); auto const offset = op.instruction.immediate; auto const unit = op.instruction.spec; auto const copy_size = (1 << unit); - auto const addr = offset + (base.holds() ? 0 : base.get()); - auto& memory = stack.proc->memory.front(); + auto const user_addr = offset + (base.holds() ? 0 : base.get()); + auto const addr = stack.proc->memory_at(user_addr); + if (addr == nullptr) { + auto o = std::ostringstream{}; + o << "invalid store address: "; + o << std::hex << std::setfill('0') << std::setw(16) << user_addr; + throw abort_execution{ip, o.str()}; + } - auto const in = get_value(registers, op.instruction.out, ip).get(); + auto const in = get_value(stack, op.instruction.out, ip).get(); switch (unit) { case 0: { auto const val = static_cast(in); - memcpy(memory.data() + addr, &val, copy_size); + memcpy(addr, &val, copy_size); break; } case 1: { auto const val = htole16(static_cast(in)); - memcpy(memory.data() + addr, &val, copy_size); + memcpy(addr, &val, copy_size); break; } case 2: { auto const val = htole32(static_cast(in)); - memcpy(memory.data() + addr, &val, copy_size); + memcpy(addr, &val, copy_size); break; } case 3: { auto const val = htole64(static_cast(in)); - memcpy(memory.data() + addr, &val, copy_size); + memcpy(addr, &val, copy_size); break; } case 4: @@ -2117,39 +2122,44 @@ auto execute(SM const op, Stack& stack, ip_type const ip) -> void } auto execute(LM const op, Stack& stack, ip_type const ip) -> void { - auto& registers = stack.frames.back().registers; - auto const base = get_value(registers, op.instruction.in, ip); + auto const base = get_value(stack, op.instruction.in, ip); auto const offset = op.instruction.immediate; auto const unit = op.instruction.spec; auto const copy_size = (1 << unit); - auto const addr = offset + (base.holds() ? 0 : base.get()); - auto& memory = stack.proc->memory.front(); + auto const user_addr = offset + (base.holds() ? 0 : base.get()); + auto const addr = stack.proc->memory_at(user_addr); + if (addr == nullptr) { + auto o = std::ostringstream{}; + o << "invalid load address: "; + o << std::hex << std::setfill('0') << std::setw(16) << user_addr; + throw abort_execution{ip, o.str()}; + } - auto out = get_proxy(registers, op.instruction.out, ip); + auto out = get_proxy(stack, op.instruction.out, ip); switch (unit) { case 0: { auto val = uint8_t{}; - memcpy(&val, memory.data() + addr, copy_size); + memcpy(&val, addr, copy_size); out = static_cast(val); break; } case 1: { auto val = uint16_t{}; - memcpy(&val, memory.data() + addr, copy_size); + memcpy(&val, addr, copy_size); out = static_cast(le16toh(val)); break; } case 2: { auto val = uint32_t{}; - memcpy(&val, memory.data() + addr, copy_size); + memcpy(&val, addr, copy_size); out = static_cast(le32toh(val)); break; } case 3: { auto val = uint64_t{}; - memcpy(&val, memory.data() + addr, copy_size); + memcpy(&val, addr, copy_size); out = le64toh(val); break; } -- 2.45.2