AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[wasm] improve handling of malformed inputs

Browse Source 
When reading malformed input, the length of variable-length types can be very large. Computing operand length with this and adding it to PC will overflow and screw up decode.

This patch switches to unsigned int for arity and lengths, terminates loop analysis on error, adds overflow checking to BranchTableOperand, and adds a unit test.

Review-Url: https://codereview.chromium.org/2052623003
Cr-Commit-Position: refs/heads/master@{#37301}
This commit is contained in:
ddchen 2016-06-27 13:35:30 -07:00 committed by Commit bot
parent ea844f9aac
commit fd2bf837a5
6 changed files with 85 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/wasm/ast-decoder.cc
View File

@ -259,7 +259,7 @@ class WasmDecoder : public Decoder {
return true;
}
int OpcodeArity(const byte* pc) {
unsigned OpcodeArity(const byte* pc) {
#define DECLARE_ARITY(name, ...) \
static const LocalType kTypes_##name[] = {__VA_ARGS__}; \
static const int kArity_##name = \
@ -340,7 +340,7 @@ class WasmDecoder : public Decoder {
}
}
int OpcodeLength(const byte* pc) {
unsigned OpcodeLength(const byte* pc) {
switch (static_cast<WasmOpcode>(*pc)) {
#define DECLARE_OPCODE_CASE(name, opcode, sig) case kExpr##name:
FOREACH_LOAD_MEM_OPCODE(DECLARE_OPCODE_CASE)
@ -621,7 +621,7 @@ class SR_WasmDecoder : public WasmDecoder {
if (pc_ >= limit_) return; // Nothing to do.
while (true) { // decoding loop.
int len = 1;
unsigned len = 1;
WasmOpcode opcode = static_cast<WasmOpcode>(*pc_);
TRACE(" @%-6d #%02x:%-20s|", startrel(pc_), opcode,
WasmOpcodes::ShortOpcodeName(opcode));
@ -1455,9 +1455,9 @@ class SR_WasmDecoder : public WasmDecoder {
new (zone_) BitVector(static_cast<int>(local_type_vec_.size()), zone_);
int depth = 0;
// Iteratively process all AST nodes nested inside the loop.
while (pc < limit_) {
while (pc < limit_ && ok()) {
WasmOpcode opcode = static_cast<WasmOpcode>(*pc);
int length = 1;
unsigned length = 1;
switch (opcode) {
case kExprLoop:
case kExprIf:
@ -1485,7 +1485,7 @@ class SR_WasmDecoder : public WasmDecoder {
if (depth <= 0) break;
pc += length;
}
return assigned;
return ok() ? assigned : nullptr;
}
inline wasm::WasmCodePosition position() {
@ -1535,12 +1535,12 @@ std::ostream& operator<<(std::ostream& os, const Tree& tree) {
return os;
}
int OpcodeLength(const byte* pc, const byte* end) {
unsigned OpcodeLength(const byte* pc, const byte* end) {
WasmDecoder decoder(nullptr, nullptr, pc, end);
return decoder.OpcodeLength(pc);
}
int OpcodeArity(const byte* pc, const byte* end) {
unsigned OpcodeArity(const byte* pc, const byte* end) {
WasmDecoder decoder(nullptr, nullptr, pc, end);
return decoder.OpcodeArity(pc);
}
@ -1588,7 +1588,7 @@ bool PrintAst(base::AccountingAllocator* allocator, const FunctionBody& body,
++line_nr;
unsigned control_depth = 0;
while (pc < body.end) {
size_t length = decoder.OpcodeLength(pc);
unsigned length = decoder.OpcodeLength(pc);
WasmOpcode opcode = static_cast<WasmOpcode>(*pc);
if (opcode == kExprElse) control_depth--;

64
src/wasm/ast-decoder.h
View File

@ -25,7 +25,7 @@ namespace wasm {
struct LocalIndexOperand {
uint32_t index;
LocalType type;
int length;
unsigned length;
inline LocalIndexOperand(Decoder* decoder, const byte* pc) {
index = decoder->checked_read_u32v(pc, 1, &length, "local index");
@ -35,7 +35,7 @@ struct LocalIndexOperand {
struct ImmI8Operand {
int8_t value;
int length;
unsigned length;
inline ImmI8Operand(Decoder* decoder, const byte* pc) {
value = bit_cast<int8_t>(decoder->checked_read_u8(pc, 1, "immi8"));
length = 1;
@ -44,7 +44,7 @@ struct ImmI8Operand {
struct ImmI32Operand {
int32_t value;
int length;
unsigned length;
inline ImmI32Operand(Decoder* decoder, const byte* pc) {
value = decoder->checked_read_i32v(pc, 1, &length, "immi32");
}
@ -52,7 +52,7 @@ struct ImmI32Operand {
struct ImmI64Operand {
int64_t value;
int length;
unsigned length;
inline ImmI64Operand(Decoder* decoder, const byte* pc) {
value = decoder->checked_read_i64v(pc, 1, &length, "immi64");
}
@ -60,7 +60,7 @@ struct ImmI64Operand {
struct ImmF32Operand {
float value;
int length;
unsigned length;
inline ImmF32Operand(Decoder* decoder, const byte* pc) {
value = bit_cast<float>(decoder->checked_read_u32(pc, 1, "immf32"));
length = 4;
@ -69,7 +69,7 @@ struct ImmF32Operand {
struct ImmF64Operand {
double value;
int length;
unsigned length;
inline ImmF64Operand(Decoder* decoder, const byte* pc) {
value = bit_cast<double>(decoder->checked_read_u64(pc, 1, "immf64"));
length = 8;
@ -80,7 +80,7 @@ struct GlobalIndexOperand {
uint32_t index;
LocalType type;
MachineType machine_type;
int length;
unsigned length;
inline GlobalIndexOperand(Decoder* decoder, const byte* pc) {
index = decoder->checked_read_u32v(pc, 1, &length, "global index");
@ -94,10 +94,10 @@ struct BreakDepthOperand {
uint32_t arity;
uint32_t depth;
Control* target;
int length;
unsigned length;
inline BreakDepthOperand(Decoder* decoder, const byte* pc) {
int len1 = 0;
int len2 = 0;
unsigned len1 = 0;
unsigned len2 = 0;
arity = decoder->checked_read_u32v(pc, 1, &len1, "argument count");
depth = decoder->checked_read_u32v(pc, 1 + len1, &len2, "break depth");
length = len1 + len2;
@ -109,10 +109,10 @@ struct CallIndirectOperand {
uint32_t arity;
uint32_t index;
FunctionSig* sig;
int length;
unsigned length;
inline CallIndirectOperand(Decoder* decoder, const byte* pc) {
int len1 = 0;
int len2 = 0;
unsigned len1 = 0;
unsigned len2 = 0;
arity = decoder->checked_read_u32v(pc, 1, &len1, "argument count");
index = decoder->checked_read_u32v(pc, 1 + len1, &len2, "signature index");
length = len1 + len2;
@ -124,10 +124,10 @@ struct CallFunctionOperand {
uint32_t arity;
uint32_t index;
FunctionSig* sig;
int length;
unsigned length;
inline CallFunctionOperand(Decoder* decoder, const byte* pc) {
int len1 = 0;
int len2 = 0;
unsigned len1 = 0;
unsigned len2 = 0;
arity = decoder->checked_read_u32v(pc, 1, &len1, "argument count");
index = decoder->checked_read_u32v(pc, 1 + len1, &len2, "function index");
length = len1 + len2;
@ -139,10 +139,10 @@ struct CallImportOperand {
uint32_t arity;
uint32_t index;
FunctionSig* sig;
int length;
unsigned length;
inline CallImportOperand(Decoder* decoder, const byte* pc) {
int len1 = 0;
int len2 = 0;
unsigned len1 = 0;
unsigned len2 = 0;
arity = decoder->checked_read_u32v(pc, 1, &len1, "argument count");
index = decoder->checked_read_u32v(pc, 1 + len1, &len2, "import index");
length = len1 + len2;
@ -154,13 +154,17 @@ struct BranchTableOperand {
uint32_t arity;
uint32_t table_count;
const byte* table;
int length;
unsigned length;
inline BranchTableOperand(Decoder* decoder, const byte* pc) {
int len1 = 0;
int len2 = 0;
unsigned len1 = 0;
unsigned len2 = 0;
arity = decoder->checked_read_u32v(pc, 1, &len1, "argument count");
table_count =
decoder->checked_read_u32v(pc, 1 + len1, &len2, "table count");
if (table_count > (UINT_MAX / sizeof(uint32_t)) - 1 ||
len1 + len2 > UINT_MAX - (table_count + 1) * sizeof(uint32_t)) {
decoder->error(pc, "branch table size overflow");
}
length = len1 + len2 + (table_count + 1) * sizeof(uint32_t);
uint32_t table_start = 1 + len1 + len2;
@ -171,8 +175,8 @@ struct BranchTableOperand {
table = nullptr;
}
}
inline uint32_t read_entry(Decoder* decoder, int i) {
DCHECK(i >= 0 && static_cast<uint32_t>(i) <= table_count);
inline uint32_t read_entry(Decoder* decoder, unsigned i) {
DCHECK(i <= table_count);
return table ? decoder->read_u32(table + i * sizeof(uint32_t)) : 0;
}
};
@ -180,12 +184,12 @@ struct BranchTableOperand {
struct MemoryAccessOperand {
uint32_t alignment;
uint32_t offset;
int length;
unsigned length;
inline MemoryAccessOperand(Decoder* decoder, const byte* pc) {
int alignment_length;
unsigned alignment_length;
alignment =
decoder->checked_read_u32v(pc, 1, &alignment_length, "alignment");
int offset_length;
unsigned offset_length;
offset = decoder->checked_read_u32v(pc, 1 + alignment_length,
&offset_length, "offset");
length = alignment_length + offset_length;
@ -194,7 +198,7 @@ struct MemoryAccessOperand {
struct ReturnArityOperand {
uint32_t arity;
int length;
unsigned length;
inline ReturnArityOperand(Decoder* decoder, const byte* pc) {
arity = decoder->checked_read_u32v(pc, 1, &length, "return count");
@ -269,10 +273,10 @@ BitVector* AnalyzeLoopAssignmentForTesting(Zone* zone, size_t num_locals,
const byte* start, const byte* end);
// Computes the length of the opcode at the given address.
int OpcodeLength(const byte* pc, const byte* end);
unsigned OpcodeLength(const byte* pc, const byte* end);
// Computes the arity (number of sub-nodes) of the opcode at the given address.
int OpcodeArity(const byte* pc, const byte* end);
unsigned OpcodeArity(const byte* pc, const byte* end);
} // namespace wasm
} // namespace internal

25
src/wasm/decoder.h
View File

@ -46,7 +46,8 @@ class Decoder {
virtual ~Decoder() {}
inline bool check(const byte* base, int offset, int length, const char* msg) {
inline bool check(const byte* base, unsigned offset, unsigned length,
const char* msg) {
DCHECK_GE(base, start_);
if ((base + offset + length) > limit_) {
error(base, base + offset, "%s", msg);
@ -56,37 +57,38 @@ class Decoder {
}
// Reads a single 8-bit byte, reporting an error if out of bounds.
inline uint8_t checked_read_u8(const byte* base, int offset,
inline uint8_t checked_read_u8(const byte* base, unsigned offset,
const char* msg = "expected 1 byte") {
return check(base, offset, 1, msg) ? base[offset] : 0;
}
// Reads 16-bit word, reporting an error if out of bounds.
inline uint16_t checked_read_u16(const byte* base, int offset,
inline uint16_t checked_read_u16(const byte* base, unsigned offset,
const char* msg = "expected 2 bytes") {
return check(base, offset, 2, msg) ? read_u16(base + offset) : 0;
}
// Reads 32-bit word, reporting an error if out of bounds.
inline uint32_t checked_read_u32(const byte* base, int offset,
inline uint32_t checked_read_u32(const byte* base, unsigned offset,
const char* msg = "expected 4 bytes") {
return check(base, offset, 4, msg) ? read_u32(base + offset) : 0;
}
// Reads 64-bit word, reporting an error if out of bounds.
inline uint64_t checked_read_u64(const byte* base, int offset,
inline uint64_t checked_read_u64(const byte* base, unsigned offset,
const char* msg = "expected 8 bytes") {
return check(base, offset, 8, msg) ? read_u64(base + offset) : 0;
}
// Reads a variable-length unsigned integer (little endian).
uint32_t checked_read_u32v(const byte* base, int offset, int* length,
uint32_t checked_read_u32v(const byte* base, unsigned offset,
unsigned* length,
const char* msg = "expected LEB32") {
return checked_read_leb<uint32_t, false>(base, offset, length, msg);
}
// Reads a variable-length signed integer (little endian).
int32_t checked_read_i32v(const byte* base, int offset, int* length,
int32_t checked_read_i32v(const byte* base, unsigned offset, unsigned* length,
const char* msg = "expected SLEB32") {
uint32_t result =
checked_read_leb<uint32_t, true>(base, offset, length, msg);
@ -100,13 +102,14 @@ class Decoder {
}
// Reads a variable-length unsigned integer (little endian).
uint64_t checked_read_u64v(const byte* base, int offset, int* length,
uint64_t checked_read_u64v(const byte* base, unsigned offset,
unsigned* length,
const char* msg = "expected LEB64") {
return checked_read_leb<uint64_t, false>(base, offset, length, msg);
}
// Reads a variable-length signed integer (little endian).
int64_t checked_read_i64v(const byte* base, int offset, int* length,
int64_t checked_read_i64v(const byte* base, unsigned offset, unsigned* length,
const char* msg = "expected SLEB64") {
uint64_t result =
checked_read_leb<uint64_t, true>(base, offset, length, msg);
@ -349,7 +352,7 @@ class Decoder {
private:
template <typename IntType, bool is_signed>
IntType checked_read_leb(const byte* base, int offset, int* length,
IntType checked_read_leb(const byte* base, unsigned offset, unsigned* length,
const char* msg) {
if (!check(base, offset, 1, msg)) {
*length = 0;
@ -370,7 +373,7 @@ class Decoder {
shift += 7;
}
DCHECK_LE(ptr - (base + offset), kMaxLength);
*length = static_cast<int>(ptr - (base + offset));
*length = static_cast<unsigned>(ptr - (base + offset));
if (ptr == end) {
// Check there are no bits set beyond the bitwidth of {IntType}.
const int kExtraBits = (1 + kMaxLength * 7) - (sizeof(IntType) * 8);

38
test/unittests/wasm/decoder-unittest.cc
View File

@ -23,7 +23,7 @@ class DecoderTest : public TestWithZone {
do { \
const byte data[] = {__VA_ARGS__}; \
decoder.Reset(data, data + sizeof(data)); \
int length; \
unsigned length; \
EXPECT_EQ(expected, \
decoder.checked_read_u32v(decoder.start(), 0, &length)); \
EXPECT_EQ(expected_length, length); \
@ -33,7 +33,7 @@ class DecoderTest : public TestWithZone {
do { \
const byte data[] = {__VA_ARGS__}; \
decoder.Reset(data, data + sizeof(data)); \
int length; \
unsigned length; \
EXPECT_EQ(expected, \
decoder.checked_read_i32v(decoder.start(), 0, &length)); \
EXPECT_EQ(expected_length, length); \
@ -43,7 +43,7 @@ class DecoderTest : public TestWithZone {
do { \
const byte data[] = {__VA_ARGS__}; \
decoder.Reset(data, data + sizeof(data)); \
int length; \
unsigned length; \
EXPECT_EQ(expected, \
decoder.checked_read_u64v(decoder.start(), 0, &length)); \
EXPECT_EQ(expected_length, length); \
@ -53,7 +53,7 @@ class DecoderTest : public TestWithZone {
do { \
const byte data[] = {__VA_ARGS__}; \
decoder.Reset(data, data + sizeof(data)); \
int length; \
unsigned length; \
EXPECT_EQ(expected, \
decoder.checked_read_i64v(decoder.start(), 0, &length)); \
EXPECT_EQ(expected_length, length); \
@ -366,7 +366,7 @@ TEST_F(DecoderTest, ReadI32v_FiveByte) {
TEST_F(DecoderTest, ReadU32v_off_end1) {
static const byte data[] = {U32V_1(11)};
int length = 0;
unsigned length = 0;
decoder.Reset(data, data);
decoder.checked_read_u32v(decoder.start(), 0, &length);
EXPECT_EQ(0, length);
@ -376,7 +376,7 @@ TEST_F(DecoderTest, ReadU32v_off_end1) {
TEST_F(DecoderTest, ReadU32v_off_end2) {
static const byte data[] = {U32V_2(1111)};
for (size_t i = 0; i < sizeof(data); i++) {
int length = 0;
unsigned length = 0;
decoder.Reset(data, data + i);
decoder.checked_read_u32v(decoder.start(), 0, &length);
EXPECT_EQ(i, length);
@ -387,7 +387,7 @@ TEST_F(DecoderTest, ReadU32v_off_end2) {
TEST_F(DecoderTest, ReadU32v_off_end3) {
static const byte data[] = {U32V_3(111111)};
for (size_t i = 0; i < sizeof(data); i++) {
int length = 0;
unsigned length = 0;
decoder.Reset(data, data + i);
decoder.checked_read_u32v(decoder.start(), 0, &length);
EXPECT_EQ(i, length);
@ -398,7 +398,7 @@ TEST_F(DecoderTest, ReadU32v_off_end3) {
TEST_F(DecoderTest, ReadU32v_off_end4) {
static const byte data[] = {U32V_4(11111111)};
for (size_t i = 0; i < sizeof(data); i++) {
int length = 0;
unsigned length = 0;
decoder.Reset(data, data + i);
decoder.checked_read_u32v(decoder.start(), 0, &length);
EXPECT_EQ(i, length);
@ -409,7 +409,7 @@ TEST_F(DecoderTest, ReadU32v_off_end4) {
TEST_F(DecoderTest, ReadU32v_off_end5) {
static const byte data[] = {U32V_5(111111111)};
for (size_t i = 0; i < sizeof(data); i++) {
int length = 0;
unsigned length = 0;
decoder.Reset(data, data + i);
decoder.checked_read_u32v(decoder.start(), 0, &length);
EXPECT_EQ(i, length);
@ -421,7 +421,7 @@ TEST_F(DecoderTest, ReadU32v_extra_bits) {
byte data[] = {0x80, 0x80, 0x80, 0x80, 0x00};
for (int i = 1; i < 16; i++) {
data[4] = static_cast<byte>(i << 4);
int length = 0;
unsigned length = 0;
decoder.Reset(data, data + sizeof(data));
decoder.checked_read_u32v(decoder.start(), 0, &length);
EXPECT_EQ(5, length);
@ -431,7 +431,7 @@ TEST_F(DecoderTest, ReadU32v_extra_bits) {
TEST_F(DecoderTest, ReadI32v_extra_bits_negative) {
// OK for negative signed values to have extra ones.
int length = 0;
unsigned length = 0;
byte data[] = {0xff, 0xff, 0xff, 0xff, 0x7f};
decoder.Reset(data, data + sizeof(data));
decoder.checked_read_i32v(decoder.start(), 0, &length);
@ -441,7 +441,7 @@ TEST_F(DecoderTest, ReadI32v_extra_bits_negative) {
TEST_F(DecoderTest, ReadI32v_extra_bits_positive) {
// Not OK for positive signed values to have extra ones.
int length = 0;
unsigned length = 0;
byte data[] = {0x80, 0x80, 0x80, 0x80, 0x77};
decoder.Reset(data, data + sizeof(data));
decoder.checked_read_i32v(decoder.start(), 0, &length);
@ -478,7 +478,7 @@ TEST_F(DecoderTest, ReadU32v_Bits) {
// foreach buffer size 0...5
for (int limit = 0; limit <= kMaxSize; limit++) {
decoder.Reset(data, data + limit);
int rlen;
unsigned rlen;
uint32_t result = decoder.checked_read_u32v(data, 0, &rlen);
if (limit < length) {
EXPECT_FALSE(decoder.ok());
@ -534,7 +534,7 @@ TEST_F(DecoderTest, ReadU64v_PowerOf2) {
for (int limit = 0; limit <= kMaxSize; limit++) {
decoder.Reset(data, data + limit);
int length;
unsigned length;
uint64_t result = decoder.checked_read_u64v(data, 0, &length);
if (limit <= index) {
EXPECT_FALSE(decoder.ok());
@ -575,7 +575,7 @@ TEST_F(DecoderTest, ReadU64v_Bits) {
// foreach buffer size 0...10
for (int limit = 0; limit <= kMaxSize; limit++) {
decoder.Reset(data, data + limit);
int rlen;
unsigned rlen;
uint64_t result = decoder.checked_read_u64v(data, 0, &rlen);
if (limit < length) {
EXPECT_FALSE(decoder.ok());
@ -617,7 +617,7 @@ TEST_F(DecoderTest, ReadI64v_Bits) {
// foreach buffer size 0...10
for (int limit = 0; limit <= kMaxSize; limit++) {
decoder.Reset(data, data + limit);
int rlen;
unsigned rlen;
int64_t result = decoder.checked_read_i64v(data, 0, &rlen);
if (limit < length) {
EXPECT_FALSE(decoder.ok());
@ -635,7 +635,7 @@ TEST_F(DecoderTest, ReadU64v_extra_bits) {
byte data[] = {0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x00};
for (int i = 1; i < 128; i++) {
data[9] = static_cast<byte>(i << 1);
int length = 0;
unsigned length = 0;
decoder.Reset(data, data + sizeof(data));
decoder.checked_read_u64v(decoder.start(), 0, &length);
EXPECT_EQ(10, length);
@ -645,7 +645,7 @@ TEST_F(DecoderTest, ReadU64v_extra_bits) {
TEST_F(DecoderTest, ReadI64v_extra_bits_negative) {
// OK for negative signed values to have extra ones.
int length = 0;
unsigned length = 0;
byte data[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f};
decoder.Reset(data, data + sizeof(data));
decoder.checked_read_i64v(decoder.start(), 0, &length);
@ -655,7 +655,7 @@ TEST_F(DecoderTest, ReadI64v_extra_bits_negative) {
TEST_F(DecoderTest, ReadI64v_extra_bits_positive) {
// Not OK for positive signed values to have extra ones.
int length = 0;
unsigned length = 0;
byte data[] = {0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x77};
decoder.Reset(data, data + sizeof(data));
decoder.checked_read_i64v(decoder.start(), 0, &length);

2
test/unittests/wasm/leb-helper-unittest.cc
View File

@ -97,7 +97,7 @@ TEST_F(LEBHelperTest, sizeof_i32v) {
EXPECT_EQ(LEBHelper::sizeof_##name(val), \
static_cast<size_t>(ptr - buffer)); \
Decoder decoder(buffer, buffer + kSize); \
int length = 0; \
unsigned length = 0; \
ctype result = decoder.checked_read_##name(buffer, 0, &length); \
EXPECT_EQ(val, result); \
EXPECT_EQ(LEBHelper::sizeof_##name(val), static_cast<size_t>(length)); \

8
test/unittests/wasm/loop-assignment-analysis-unittest.cc
View File

@ -180,6 +180,14 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, Loop2) {
}
}
TEST_F(WasmLoopAssignmentAnalyzerTest, Malformed) {
byte code[] = {kExprLoop, kExprF32Neg, kExprBrTable, 0x0e, 'h', 'e',
'l', 'l', 'o', ',', ' ', 'w',
'o', 'r', 'l', 'd', '!'};
BitVector* assigned = Analyze(code, code + arraysize(code));
CHECK_NULL(assigned);
}
} // namespace wasm
} // namespace internal
} // namespace v8