Skip to content

Commit 38c728b

Browse files
author
Evgeniy Gorbanyov
committed
wasi: fix integer overflow in iovec bounds checks
Guest-controlled counts were multiplied by the serialized element size in uint32_t. A length such as 0x20000000 wraps the product to 0, so the bounds check accepts the array and deserialization reads past the end of the WebAssembly memory. Multiply in size_t and reject an overflowing product before reading the array. The same check covers fd_read, fd_write, fd_pread, fd_pwrite, poll_oneoff, sock_recv, and sock_send. Assisted-by: Grok 4.7 Signed-off-by: Evgeniy Gorbanyov <esgor@altlinux.org>
1 parent 2052d0d commit 38c728b

2 files changed

Lines changed: 66 additions & 16 deletions

File tree

‎src/node_wasi.cc‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,18 @@ inline void Debug(const WASI& wasi, Args&&... args) {
2626
} \
2727
} while (0)
2828

29+
// elem_size * count is computed in size_t. A uint32_t product wraps
30+
// (for example 0x20000000 * 8 == 0) and then skips the bounds check.
31+
#define CHECK_ARRAY_BOUNDS_OR_RETURN(mem_size, offset, elem_size, count) \
32+
do { \
33+
if (!uvwasi_serdes_check_array_bounds((offset), \
34+
(mem_size), \
35+
(elem_size), \
36+
(count))) { \
37+
return UVWASI_EOVERFLOW; \
38+
} \
39+
} while (0)
40+
2941
using v8::Array;
3042
using v8::ArrayBuffer;
3143
using v8::BigInt;
@@ -664,8 +676,8 @@ uint32_t WASI::FdPread(WASI& wasi,
664676
iovs_len,
665677
offset,
666678
nread_ptr);
667-
CHECK_BOUNDS_OR_RETURN(
668-
memory.size, iovs_ptr, iovs_len * UVWASI_SERDES_SIZE_iovec_t);
679+
CHECK_ARRAY_BOUNDS_OR_RETURN(
680+
memory.size, iovs_ptr, UVWASI_SERDES_SIZE_iovec_t, iovs_len);
669681
CHECK_BOUNDS_OR_RETURN(memory.size, nread_ptr, UVWASI_SERDES_SIZE_size_t);
670682
std::vector<uvwasi_iovec_t> iovs(iovs_len);
671683
uvwasi_errno_t err;
@@ -724,8 +736,8 @@ uint32_t WASI::FdPwrite(WASI& wasi,
724736
iovs_len,
725737
offset,
726738
nwritten_ptr);
727-
CHECK_BOUNDS_OR_RETURN(
728-
memory.size, iovs_ptr, iovs_len * UVWASI_SERDES_SIZE_ciovec_t);
739+
CHECK_ARRAY_BOUNDS_OR_RETURN(
740+
memory.size, iovs_ptr, UVWASI_SERDES_SIZE_ciovec_t, iovs_len);
729741
CHECK_BOUNDS_OR_RETURN(memory.size, nwritten_ptr, UVWASI_SERDES_SIZE_size_t);
730742
std::vector<uvwasi_ciovec_t> iovs(iovs_len);
731743
uvwasi_errno_t err;
@@ -752,8 +764,8 @@ uint32_t WASI::FdRead(WASI& wasi,
752764
uint32_t iovs_len,
753765
uint32_t nread_ptr) {
754766
Debug(wasi, "fd_read(%d, %d, %d, %d)\n", fd, iovs_ptr, iovs_len, nread_ptr);
755-
CHECK_BOUNDS_OR_RETURN(
756-
memory.size, iovs_ptr, iovs_len * UVWASI_SERDES_SIZE_iovec_t);
767+
CHECK_ARRAY_BOUNDS_OR_RETURN(
768+
memory.size, iovs_ptr, UVWASI_SERDES_SIZE_iovec_t, iovs_len);
757769
CHECK_BOUNDS_OR_RETURN(memory.size, nread_ptr, UVWASI_SERDES_SIZE_size_t);
758770
std::vector<uvwasi_iovec_t> iovs(iovs_len);
759771
uvwasi_errno_t err;
@@ -853,8 +865,8 @@ uint32_t WASI::FdWrite(WASI& wasi,
853865
iovs_ptr,
854866
iovs_len,
855867
nwritten_ptr);
856-
CHECK_BOUNDS_OR_RETURN(
857-
memory.size, iovs_ptr, iovs_len * UVWASI_SERDES_SIZE_ciovec_t);
868+
CHECK_ARRAY_BOUNDS_OR_RETURN(
869+
memory.size, iovs_ptr, UVWASI_SERDES_SIZE_ciovec_t, iovs_len);
858870
CHECK_BOUNDS_OR_RETURN(memory.size, nwritten_ptr, UVWASI_SERDES_SIZE_size_t);
859871
std::vector<uvwasi_ciovec_t> iovs(iovs_len);
860872
uvwasi_errno_t err;
@@ -1126,10 +1138,12 @@ uint32_t WASI::PollOneoff(WASI& wasi,
11261138
out_ptr,
11271139
nsubscriptions,
11281140
nevents_ptr);
1129-
CHECK_BOUNDS_OR_RETURN(
1130-
memory.size, in_ptr, nsubscriptions * UVWASI_SERDES_SIZE_subscription_t);
1131-
CHECK_BOUNDS_OR_RETURN(
1132-
memory.size, out_ptr, nsubscriptions * UVWASI_SERDES_SIZE_event_t);
1141+
CHECK_ARRAY_BOUNDS_OR_RETURN(memory.size,
1142+
in_ptr,
1143+
UVWASI_SERDES_SIZE_subscription_t,
1144+
nsubscriptions);
1145+
CHECK_ARRAY_BOUNDS_OR_RETURN(
1146+
memory.size, out_ptr, UVWASI_SERDES_SIZE_event_t, nsubscriptions);
11331147
CHECK_BOUNDS_OR_RETURN(memory.size, nevents_ptr, UVWASI_SERDES_SIZE_size_t);
11341148
std::vector<uvwasi_subscription_t> in(nsubscriptions);
11351149
std::vector<uvwasi_event_t> out(nsubscriptions);
@@ -1209,8 +1223,8 @@ uint32_t WASI::SockRecv(WASI& wasi,
12091223
ri_flags,
12101224
ro_datalen_ptr,
12111225
ro_flags_ptr);
1212-
CHECK_BOUNDS_OR_RETURN(
1213-
memory.size, ri_data_ptr, ri_data_len * UVWASI_SERDES_SIZE_iovec_t);
1226+
CHECK_ARRAY_BOUNDS_OR_RETURN(
1227+
memory.size, ri_data_ptr, UVWASI_SERDES_SIZE_iovec_t, ri_data_len);
12141228
CHECK_BOUNDS_OR_RETURN(memory.size, ro_datalen_ptr, 4);
12151229
CHECK_BOUNDS_OR_RETURN(memory.size, ro_flags_ptr, 4);
12161230
std::vector<uvwasi_iovec_t> ri_data(ri_data_len);
@@ -1251,8 +1265,8 @@ uint32_t WASI::SockSend(WASI& wasi,
12511265
si_data_len,
12521266
si_flags,
12531267
so_datalen_ptr);
1254-
CHECK_BOUNDS_OR_RETURN(
1255-
memory.size, si_data_ptr, si_data_len * UVWASI_SERDES_SIZE_ciovec_t);
1268+
CHECK_ARRAY_BOUNDS_OR_RETURN(
1269+
memory.size, si_data_ptr, UVWASI_SERDES_SIZE_ciovec_t, si_data_len);
12561270
CHECK_BOUNDS_OR_RETURN(
12571271
memory.size, so_datalen_ptr, UVWASI_SERDES_SIZE_size_t);
12581272
std::vector<uvwasi_ciovec_t> si_data(si_data_len);

‎test/wasi/test-wasi-iovec-overflow.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const { WASI } = require('wasi');
5+
6+
// deps/uvwasi/include/wasi_types.h
7+
const UVWASI_ESUCCESS = 0;
8+
const UVWASI_EOVERFLOW = 61;
9+
10+
// _start calls fd_read and stores each errno:
11+
// 0: iovs_len 0x20000000 (0x20000000 * 8 wraps uint32_t to 0)
12+
// 4: iovs_len 0x20000001 (wraps to 8, which fits in one page)
13+
// 8: iovs_len 0
14+
const bytes = Uint8Array.from([
15+
0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00, 0x01, 0x0c, 0x02, 0x60, 0x00, 0x00, 0x60, 0x04,
16+
0x7f, 0x7f, 0x7f, 0x7f, 0x01, 0x7f, 0x02, 0x22, 0x01, 0x16, 0x77, 0x61, 0x73, 0x69, 0x5f, 0x73,
17+
0x6e, 0x61, 0x70, 0x73, 0x68, 0x6f, 0x74, 0x5f, 0x70, 0x72, 0x65, 0x76, 0x69, 0x65, 0x77, 0x31,
18+
0x07, 0x66, 0x64, 0x5f, 0x72, 0x65, 0x61, 0x64, 0x00, 0x01, 0x03, 0x02, 0x01, 0x00, 0x05, 0x03,
19+
0x01, 0x00, 0x01, 0x07, 0x13, 0x02, 0x06, 0x6d, 0x65, 0x6d, 0x6f, 0x72, 0x79, 0x02, 0x00, 0x06,
20+
0x5f, 0x73, 0x74, 0x61, 0x72, 0x74, 0x00, 0x01, 0x0a, 0x39, 0x01, 0x37, 0x00, 0x41, 0x00, 0x41,
21+
0x00, 0x41, 0x00, 0x41, 0x80, 0x80, 0x80, 0x80, 0x02, 0x41, 0x10, 0x10, 0x00, 0x36, 0x02, 0x00,
22+
0x41, 0x04, 0x41, 0x00, 0x41, 0x00, 0x41, 0x81, 0x80, 0x80, 0x80, 0x02, 0x41, 0x10, 0x10, 0x00,
23+
0x36, 0x02, 0x00, 0x41, 0x08, 0x41, 0x00, 0x41, 0x00, 0x41, 0x00, 0x41, 0x10, 0x10, 0x00, 0x36,
24+
0x02, 0x00, 0x0b,
25+
]);
26+
27+
(async () => {
28+
const wasi = new WASI({ version: 'preview1', returnOnExit: true });
29+
const { instance } = await WebAssembly.instantiate(bytes, wasi.getImportObject());
30+
assert.strictEqual(wasi.start(instance), 0);
31+
32+
const view = new DataView(instance.exports.memory.buffer);
33+
assert.strictEqual(view.getUint32(0, true), UVWASI_EOVERFLOW);
34+
assert.strictEqual(view.getUint32(4, true), UVWASI_EOVERFLOW);
35+
assert.strictEqual(view.getUint32(8, true), UVWASI_ESUCCESS);
36+
})().then(common.mustCall());

0 commit comments

Comments
 (0)