Skip to content

Commit c5b39ec

Browse files
PiotrSikoraknm3000
authored andcommitted
v8: add restricted callbacks. (proxy-wasm#306)
Restricted callbacks can invoke only allow-listed hostcalls. This mechanism is introduced primarily to prevent malicious plugins from modifying the state from "proxy_on_memory_allocate" ("malloc") callbacks, which could result in dangling pointers and/or out-of-bound access. While there, limit the hostcalls available during early initialization ("_initialize", "_start", and "main" callbacks). Reported by Chris Ertl from Google Security. Signed-off-by: Piotr Sikora <[email protected]>
1 parent 1584ba1 commit c5b39ec

File tree

8 files changed

+228
-0
lines changed

8 files changed

+228
-0
lines changed

include/proxy-wasm/wasm.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,15 @@ inline void *WasmBase::allocMemory(uint64_t size, uint64_t *address) {
392392
if (!malloc_) {
393393
return nullptr;
394394
}
395+
wasm_vm_->setRestrictedCallback(
396+
true, {// logging (Proxy-Wasm)
397+
"env.proxy_log",
398+
// logging (stdout/stderr)
399+
"wasi_unstable.fd_write", "wasi_snapshot_preview1.fd_write",
400+
// time
401+
"wasi_unstable.clock_time_get", "wasi_snapshot_preview1.clock_time_get"});
395402
Word a = malloc_(vm_context(), size);
403+
wasm_vm_->setRestrictedCallback(false);
396404
if (!a.u64_) {
397405
return nullptr;
398406
}

include/proxy-wasm/wasm_vm.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <optional>
2121
#include <string>
2222
#include <unordered_map>
23+
#include <unordered_set>
2324
#include <vector>
2425

2526
#include "include/proxy-wasm/word.h"
@@ -314,6 +315,16 @@ class WasmVm {
314315
fail_callbacks_.push_back(fail_callback);
315316
}
316317

318+
bool isHostFunctionAllowed(const std::string &name) {
319+
return !restricted_callback_ || allowed_hostcalls_.find(name) != allowed_hostcalls_.end();
320+
}
321+
322+
void setRestrictedCallback(bool restricted,
323+
std::unordered_set<std::string> allowed_hostcalls = {}) {
324+
restricted_callback_ = restricted;
325+
allowed_hostcalls_ = std::move(allowed_hostcalls);
326+
}
327+
317328
// Integrator operations.
318329
std::unique_ptr<WasmVmIntegration> &integration() { return integration_; }
319330
bool cmpLogLevel(proxy_wasm::LogLevel level) { return integration_->getLogLevel() <= level; }
@@ -322,6 +333,10 @@ class WasmVm {
322333
std::unique_ptr<WasmVmIntegration> integration_;
323334
FailState failed_ = FailState::Ok;
324335
std::vector<std::function<void(FailState)>> fail_callbacks_;
336+
337+
private:
338+
bool restricted_callback_{false};
339+
std::unordered_set<std::string> allowed_hostcalls_{};
325340
};
326341

327342
// Thread local state set during a call into a WASM VM so that calls coming out of the

src/v8/v8.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ class V8 : public WasmVm {
107107
void terminate() override;
108108

109109
private:
110+
wasm::own<wasm::Trap> trap(std::string message);
111+
110112
std::string getFailMessage(std::string_view function_name, wasm::own<wasm::Trap> trap);
111113

112114
template <typename... Args>
@@ -503,6 +505,10 @@ bool V8::setWord(uint64_t pointer, Word word) {
503505
return true;
504506
}
505507

508+
wasm::own<wasm::Trap> V8::trap(std::string message) {
509+
return wasm::Trap::make(store_.get(), wasm::Message::make(std::move(message)));
510+
}
511+
506512
template <typename... Args>
507513
void V8::registerHostFunctionImpl(std::string_view module_name, std::string_view function_name,
508514
void (*function)(Args...)) {
@@ -519,6 +525,9 @@ void V8::registerHostFunctionImpl(std::string_view module_name, std::string_view
519525
func_data->vm_->integration()->trace("[vm->host] " + func_data->name_ + "(" +
520526
printValues(params, sizeof...(Args)) + ")");
521527
}
528+
if (!func_data->vm_->isHostFunctionAllowed(func_data->name_)) {
529+
return dynamic_cast<V8 *>(func_data->vm_)->trap("restricted_callback");
530+
}
522531
auto args = convertValTypesToArgsTuple<std::tuple<Args...>>(params);
523532
auto function = reinterpret_cast<void (*)(Args...)>(func_data->raw_func_);
524533
std::apply(function, args);
@@ -552,6 +561,9 @@ void V8::registerHostFunctionImpl(std::string_view module_name, std::string_view
552561
func_data->vm_->integration()->trace("[vm->host] " + func_data->name_ + "(" +
553562
printValues(params, sizeof...(Args)) + ")");
554563
}
564+
if (!func_data->vm_->isHostFunctionAllowed(func_data->name_)) {
565+
return dynamic_cast<V8 *>(func_data->vm_)->trap("restricted_callback");
566+
}
555567
auto args = convertValTypesToArgsTuple<std::tuple<Args...>>(params);
556568
auto function = reinterpret_cast<R (*)(Args...)>(func_data->raw_func_);
557569
R rvalue = std::apply(function, args);

src/wasm.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,25 @@ ContextBase *WasmBase::getRootContext(const std::shared_ptr<PluginBase> &plugin,
355355
}
356356

357357
void WasmBase::startVm(ContextBase *root_context) {
358+
// wasi_snapshot_preview1.clock_time_get
359+
wasm_vm_->setRestrictedCallback(
360+
true, {// logging (Proxy-Wasm)
361+
"env.proxy_log",
362+
// logging (stdout/stderr)
363+
"wasi_unstable.fd_write", "wasi_snapshot_preview1.fd_write",
364+
// args
365+
"wasi_unstable.args_sizes_get", "wasi_snapshot_preview1.args_sizes_get",
366+
"wasi_unstable.args_get", "wasi_snapshot_preview1.args_get",
367+
// environment variables
368+
"wasi_unstable.environ_sizes_get", "wasi_snapshot_preview1.environ_sizes_get",
369+
"wasi_unstable.environ_get", "wasi_snapshot_preview1.environ_get",
370+
// preopened files/directories
371+
"wasi_unstable.fd_prestat_get", "wasi_snapshot_preview1.fd_prestat_get",
372+
"wasi_unstable.fd_prestat_dir_name", "wasi_snapshot_preview1.fd_prestat_dir_name",
373+
// time
374+
"wasi_unstable.clock_time_get", "wasi_snapshot_preview1.clock_time_get",
375+
// random
376+
"wasi_unstable.random_get", "wasi_snapshot_preview1.random_get"});
358377
if (_initialize_) {
359378
// WASI reactor.
360379
_initialize_(root_context);
@@ -370,6 +389,7 @@ void WasmBase::startVm(ContextBase *root_context) {
370389
// WASI command.
371390
_start_(root_context);
372391
}
392+
wasm_vm_->setRestrictedCallback(false);
373393
}
374394

375395
bool WasmBase::configure(ContextBase *root_context, std::shared_ptr<PluginBase> plugin) {

test/BUILD

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,21 @@ cc_test(
102102
],
103103
)
104104

105+
cc_test(
106+
name = "security_test",
107+
srcs = ["security_test.cc"],
108+
data = [
109+
"//test/test_data:bad_malloc.wasm",
110+
],
111+
linkstatic = 1,
112+
deps = [
113+
":utility_lib",
114+
"//:lib",
115+
"@com_google_googletest//:gtest",
116+
"@com_google_googletest//:gtest_main",
117+
],
118+
)
119+
105120
cc_test(
106121
name = "shared_data",
107122
srcs = ["shared_data_test.cc"],

test/security_test.cc

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#include "gtest/gtest.h"
16+
17+
#include <memory>
18+
#include <string>
19+
20+
#include "include/proxy-wasm/context.h"
21+
#include "include/proxy-wasm/wasm.h"
22+
23+
#include "test/utility.h"
24+
25+
namespace proxy_wasm {
26+
27+
INSTANTIATE_TEST_SUITE_P(WasmEngines, TestVm, testing::ValuesIn(getWasmEngines()),
28+
[](const testing::TestParamInfo<std::string> &info) {
29+
return info.param;
30+
});
31+
32+
TEST_P(TestVm, MallocNoHostcalls) {
33+
if (engine_ != "v8") {
34+
return;
35+
}
36+
auto source = readTestWasmFile("bad_malloc.wasm");
37+
ASSERT_FALSE(source.empty());
38+
auto wasm = TestWasm(std::move(vm_));
39+
ASSERT_TRUE(wasm.load(source, false));
40+
ASSERT_TRUE(wasm.initialize());
41+
42+
uint64_t ptr = 0;
43+
void *result = wasm.allocMemory(0x1000, &ptr);
44+
EXPECT_NE(result, nullptr);
45+
EXPECT_FALSE(wasm.isFailed());
46+
47+
// Check application logs.
48+
auto *context = dynamic_cast<TestContext *>(wasm.vm_context());
49+
EXPECT_TRUE(context->isLogEmpty());
50+
// Check integration logs.
51+
auto *integration = dynamic_cast<TestIntegration *>(wasm.wasm_vm()->integration().get());
52+
EXPECT_FALSE(integration->isErrorLogged("Function: proxy_on_memory_allocate failed"));
53+
EXPECT_FALSE(integration->isErrorLogged("restricted_callback"));
54+
}
55+
56+
TEST_P(TestVm, MallocWithLog) {
57+
if (engine_ != "v8") {
58+
return;
59+
}
60+
auto source = readTestWasmFile("bad_malloc.wasm");
61+
ASSERT_FALSE(source.empty());
62+
auto wasm = TestWasm(std::move(vm_));
63+
ASSERT_TRUE(wasm.load(source, false));
64+
ASSERT_TRUE(wasm.initialize());
65+
66+
uint64_t ptr = 0;
67+
// 0xAAAA => hostcall to proxy_log (allowed).
68+
void *result = wasm.allocMemory(0xAAAA, &ptr);
69+
EXPECT_NE(result, nullptr);
70+
EXPECT_FALSE(wasm.isFailed());
71+
72+
// Check application logs.
73+
auto *context = dynamic_cast<TestContext *>(wasm.vm_context());
74+
EXPECT_TRUE(context->isLogged("this is fine"));
75+
// Check integration logs.
76+
auto *integration = dynamic_cast<TestIntegration *>(wasm.wasm_vm()->integration().get());
77+
EXPECT_FALSE(integration->isErrorLogged("Function: proxy_on_memory_allocate failed"));
78+
EXPECT_FALSE(integration->isErrorLogged("restricted_callback"));
79+
}
80+
81+
TEST_P(TestVm, MallocWithHostcall) {
82+
if (engine_ != "v8") {
83+
return;
84+
}
85+
auto source = readTestWasmFile("bad_malloc.wasm");
86+
ASSERT_FALSE(source.empty());
87+
auto wasm = TestWasm(std::move(vm_));
88+
ASSERT_TRUE(wasm.load(source, false));
89+
ASSERT_TRUE(wasm.initialize());
90+
91+
uint64_t ptr = 0;
92+
// 0xBBBB => hostcall to proxy_done (not allowed).
93+
void *result = wasm.allocMemory(0xBBBB, &ptr);
94+
EXPECT_EQ(result, nullptr);
95+
EXPECT_TRUE(wasm.isFailed());
96+
97+
// Check application logs.
98+
auto *context = dynamic_cast<TestContext *>(wasm.vm_context());
99+
EXPECT_TRUE(context->isLogEmpty());
100+
// Check integration logs.
101+
auto *integration = dynamic_cast<TestIntegration *>(wasm.wasm_vm()->integration().get());
102+
EXPECT_TRUE(integration->isErrorLogged("Function: proxy_on_memory_allocate failed"));
103+
EXPECT_TRUE(integration->isErrorLogged("restricted_callback"));
104+
}
105+
106+
} // namespace proxy_wasm

test/test_data/BUILD

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ wasm_rust_binary(
2828
],
2929
)
3030

31+
wasm_rust_binary(
32+
name = "bad_malloc.wasm",
33+
srcs = ["bad_malloc.rs"],
34+
)
35+
3136
wasm_rust_binary(
3237
name = "callback.wasm",
3338
srcs = ["callback.rs"],

test/test_data/bad_malloc.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use std::mem::MaybeUninit;
16+
17+
extern "C" {
18+
fn proxy_log(level: u32, message_data: *const u8, message_size: usize) -> u32;
19+
fn proxy_done() -> u32;
20+
}
21+
22+
#[no_mangle]
23+
pub extern "C" fn proxy_abi_version_0_2_0() {}
24+
25+
#[no_mangle]
26+
pub extern "C" fn proxy_on_memory_allocate(size: usize) -> *mut u8 {
27+
let mut vec: Vec<MaybeUninit<u8>> = Vec::with_capacity(size);
28+
unsafe {
29+
vec.set_len(size);
30+
}
31+
match size {
32+
0xAAAA => {
33+
let message = "this is fine";
34+
unsafe {
35+
proxy_log(0, message.as_ptr(), message.len());
36+
}
37+
}
38+
0xBBBB => {
39+
unsafe {
40+
proxy_done();
41+
}
42+
}
43+
_ => {}
44+
}
45+
let slice = vec.into_boxed_slice();
46+
Box::into_raw(slice) as *mut u8
47+
}

0 commit comments

Comments
 (0)