authorgravatar for malcolm.still@gmail.comMalcolm Still <malcolm.still@gmail.com> 2021-09-19 20:17:01+01:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2021-09-20 01:58:18-04:00
log1f61076ffb3027e043ff78995344267b39b6226f
treea5d8ea3a9adbb81cd33e787abc79c421421dd60f
parent9fa723ee5043a9d2cb017e417f2e27041f671146

I'm working on a WebAssembly interpreter in zig. WebAssembly uses LEB128 encoding throughout its specification.

The WebAssembly spec requires signed LEB128 to be encoded up to a maximum number of bytes (max 5 bytes for i32, max 10 bytes for i64) and that "unused" bits are all 0 if the number is positive and all 1 if the number is negative. The Zig LEB128 implementation already enforces the max number of bytes and does check the unused bytes https://github.com/ziglang/zig/blob/master/lib/std/leb128.zig#L70-L79. However, the WebAssembly test suite has a number of tests that were failing validation (expecting the wasm module to fail validation, but when running the tests, those examples were actually passing validation): https://github.com/malcolmstill/foxwren/blob/master/test/testsuite/binary-leb128.wast#L893-L902 https://github.com/malcolmstill/foxwren/blob/master/test/testsuite/binary-leb128.wast#L934-L943 Notably the failures are both cases of negative numbers and the top 4 bits of the last byte are zero. And I believe this is the issue: we're only currently checking the "unused" / remaining_bits if we overflow, but in the case of 0x0_ no overflow happens and so the bits go unchecked. In other words: \xff\xff\xff\xff\7f rightly successfully decodes (because it overflows and the remaining bits are 0b1111) \xff\xff\xff\xff\6f rightly errors with overflow (because it overflows and the remaining bits are 0b1110) \xff\xff\xff\xff\0f incorrectly decodes when it should error (because the top 4 bits are all 0, and so no overflow occurs and no check that the unused bits are 1 happens) This PR adds a the remaining_bits check in an else branch of the @shlWithOverflow when we're looking at the last byte and the number being decoded is negative. Note: this means a couple of the test cases in leb128.zig that are down as decoding shouldn't actually decode so I added the appropriate 1 bits.

1 files changed, 12 insertions(+), 2 deletions(-)

lib/std/leb128.zig+12-2
......@@ -76,6 +76,14 @@ pub fn readILEB128(comptime T: type, reader: anytype) !T {
7676 const remaining_shift = @intCast(u3, @typeInfo(U).Int.bits - @as(u16, shift));
7777 const remaining_bits = @bitCast(i8, byte | 0x80) >> remaining_shift;
7878 if (remaining_bits != -1) return error.Overflow;
79 } else {
80 // If we don't overflow and this is the last byte and the number being decoded
81 // is negative, check that the remaining bits are 1
82 if ((byte & 0x80 == 0) and (@bitCast(S, temp) < 0)) {
83 const remaining_shift = @intCast(u3, @typeInfo(U).Int.bits - @as(u16, shift));
84 const remaining_bits = @bitCast(i8, byte | 0x80) >> remaining_shift;
85 if (remaining_bits != -1) return error.Overflow;
86 }
7987 }
8088
8189 value |= temp;
......@@ -215,6 +223,8 @@ test "deserialize signed LEB128" {
215223 try testing.expectError(error.Overflow, test_read_ileb128(i32, "\x80\x80\x80\x80\x40"));
216224 try testing.expectError(error.Overflow, test_read_ileb128(i64, "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x40"));
217225 try testing.expectError(error.Overflow, test_read_ileb128(i8, "\xff\x7e"));
226 try testing.expectError(error.Overflow, test_read_ileb128(i32, "\x80\x80\x80\x80\x08"));
227 try testing.expectError(error.Overflow, test_read_ileb128(i64, "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x01"));
218228
219229 // Decode SLEB128
220230 try testing.expect((try test_read_ileb128(i64, "\x00")) == 0);
......@@ -233,8 +243,8 @@ test "deserialize signed LEB128" {
233243 try testing.expect((try test_read_ileb128(i8, "\xff\x7f")) == -1);
234244 try testing.expect((try test_read_ileb128(i16, "\xff\xff\x7f")) == -1);
235245 try testing.expect((try test_read_ileb128(i32, "\xff\xff\xff\xff\x7f")) == -1);
236 try testing.expect((try test_read_ileb128(i32, "\x80\x80\x80\x80\x08")) == -0x80000000);
237 try testing.expect((try test_read_ileb128(i64, "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x01")) == @bitCast(i64, @intCast(u64, 0x8000000000000000)));
246 try testing.expect((try test_read_ileb128(i32, "\x80\x80\x80\x80\x78")) == -0x80000000);
247 try testing.expect((try test_read_ileb128(i64, "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x7f")) == @bitCast(i64, @intCast(u64, 0x8000000000000000)));
238248 try testing.expect((try test_read_ileb128(i64, "\x80\x80\x80\x80\x80\x80\x80\x80\x40")) == -0x4000000000000000);
239249 try testing.expect((try test_read_ileb128(i64, "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x7f")) == -0x8000000000000000);
240250