[wali] Fix sundry out-of-bounds bugs

src/util/FileDescriptorMap.v3

@@ -23,9 +23,19 @@ class FileDescriptorMap(max: int) {
 		}
 		return -1;
 	}
-	def set(fd: int, sys: int) {
+	// Binds a user-level {fd} to the system-level {sys}. Returns {false} if {fd} is out
+	// of range; {fd} may come straight from a guest (e.g. the newfd of dup2), and an
+	// unchecked store here would let a guest crash the engine with a bounds check.
+	def set(fd: int, sys: int) -> bool {
+		if (fd < 0 || fd >= fds.length) return false;
 		fds[fd] = sys;
 		sysfd_to_userfd[sys] = fd;
+		return true;
+	}
+	// Checks whether {fd} is a legal index into this table, independent of whether it
+	// is currently bound.
+	def in_range(fd: int) -> bool {
+		return fd >= 0 && fd < fds.length;
 	}
 	def alloc() -> int {
 		for (i = lastfd; i < fds.length; i++) {

test/wali/Dup2Bad.v3

@@ -0,0 +1,28 @@
+// dup2/dup3 take the new fd straight from the guest and use it to index the engine's
+// fd table. An out-of-range value must be rejected with -EBADF; an unchecked store
+// there lets a guest crash the engine with a bounds check exception.
+import component wali {
+	def SYS_write(fd: i32, buf: Pointer, count: i32) -> i64;
+	def SYS_dup2(oldfd: i32, newfd: i32) -> i64;
+	def SYS_dup3(oldfd: i32, newfd: i32, flags: i32) -> i64;
+}
+
+def STDOUT = 1;
+def EBADF: i64 = -9;
+
+def main() -> i64 {
+	check("dup2 negative newfd", wali.SYS_dup2(STDOUT, -1) == EBADF);
+	check("dup2 newfd past table", wali.SYS_dup2(STDOUT, 0x7fffffff) == EBADF);
+	check("dup3 negative newfd", wali.SYS_dup3(STDOUT, -1, 0) == EBADF);
+	check("dup3 newfd past table", wali.SYS_dup3(STDOUT, 0x7fffffff, 0) == EBADF);
+	return 0;
+}
+
+def check(name: string, cond: bool) {
+	print(name);
+	print(if(cond, ": ok\n", ": FAIL\n"));
+}
+
+def print(str: string) {
+	wali.SYS_write(STDOUT, Pointer.atContents(str), str.length);
+}

test/wali/Dup2Bad.wasm.exit

@@ -0,0 +1 @@
+0

test/wali/Dup2Bad.wasm.out

@@ -0,0 +1,4 @@
+dup2 negative newfd: ok
+dup2 newfd past table: ok
+dup3 negative newfd: ok
+dup3 newfd past table: ok

test/wali/Enosys.v3

@@ -0,0 +1,31 @@
+// Unimplemented WALI syscalls must report -ENOSYS. The Linux syscall ABI signals
+// failure with a *negative* errno; musl's __syscall_ret() treats any result outside
+// [-4095, -1] as success. Returning a positive ENOSYS makes the guest believe the call
+// succeeded and returned 38.
+import component wali {
+	def SYS_write(fd: i32, buf: Pointer, count: i32) -> i64;
+	def SYS_writev(fd: i32, iov: Pointer, iovcnt: i32) -> i64;
+	def SYS_readv(fd: i32, iov: Pointer, iovcnt: i32) -> i64;
+	def SYS_munmap(addr: Pointer, len: i32) -> i64;
+	def SYS_exit_group(status: i32) -> i64;
+}
+
+def STDOUT = 1;
+def ENOSYS: i64 = -38;
+
+def main() -> i64 {
+	check("writev", wali.SYS_writev(STDOUT, Pointer.NULL, 0) == ENOSYS);
+	check("readv", wali.SYS_readv(0, Pointer.NULL, 0) == ENOSYS);
+	check("munmap", wali.SYS_munmap(Pointer.NULL, 4096) == ENOSYS);
+	check("exit_group", wali.SYS_exit_group(0) == ENOSYS);
+	return 0;
+}
+
+def check(name: string, cond: bool) {
+	print(name);
+	print(if(cond, ": ok\n", ": FAIL\n"));
+}
+
+def print(str: string) {
+	wali.SYS_write(STDOUT, Pointer.atContents(str), str.length);
+}

test/wali/Enosys.wasm.exit

@@ -0,0 +1 @@
+0

test/wali/Enosys.wasm.out

@@ -0,0 +1,4 @@
+writev: ok
+readv: ok
+munmap: ok
+exit_group: ok

test/wali/Ioctl.v3

@@ -0,0 +1,38 @@
+// WALI defines ioctl's third argument as a guest pointer for every request, so it has
+// to be translated. Forwarding the guest offset to the kernel as a host address faults.
+import component wali {
+	def SYS_write(fd: i32, buf: Pointer, count: i32) -> i64;
+	def SYS_close(fd: i32) -> i64;
+	def SYS_socketpair(domain: i32, stype: i32, protocol: i32, sv: Pointer) -> i64;
+	def SYS_ioctl(fd: i32, request: i32, arg: Pointer) -> i64;
+}
+
+def STDOUT = 1;
+def AF_UNIX = 1;
+def SOCK_STREAM = 1;
+def FIONREAD = 0x541B;
+
+def main() -> i64 {
+	var sv = Array<int>.new(2);
+	check("socketpair", wali.SYS_socketpair(AF_UNIX, SOCK_STREAM, 0, Pointer.atContents(sv)) == 0);
+
+	var msg = "hello";
+	check("write 5 bytes", wali.SYS_write(sv[1], Pointer.atContents(msg), msg.length) == 5);
+
+	var avail = Array<int>.new(1);
+	check("ioctl FIONREAD", wali.SYS_ioctl(sv[0], FIONREAD, Pointer.atContents(avail)) == 0);
+	check("FIONREAD reports 5", avail[0] == 5);
+
+	wali.SYS_close(sv[0]);
+	wali.SYS_close(sv[1]);
+	return 0;
+}
+
+def check(name: string, cond: bool) {
+	print(name);
+	print(if(cond, ": ok\n", ": FAIL\n"));
+}
+
+def print(str: string) {
+	wali.SYS_write(STDOUT, Pointer.atContents(str), str.length);
+}

test/wali/Ioctl.wasm.exit

@@ -0,0 +1 @@
+0

test/wali/Ioctl.wasm.out

@@ -0,0 +1,4 @@
+socketpair: ok
+write 5 bytes: ok
+ioctl FIONREAD: ok
+FIONREAD reports 5: ok

test/wali/MmapFile.v3

@@ -0,0 +1,44 @@
+// A file-backed mmap has to land inside linear memory at the offset returned to the
+// guest. The mapping address is derived from the memory's base; computing it from the
+// memory's *length* instead maps the file at an unrelated host address, so the guest
+// reads nothing useful at the returned offset (and the engine's own address space is
+// mapped over).
+import component wali {
+	def SYS_open(pathname: Pointer, flags: i32, mode: i32) -> i64;
+	def SYS_close(fd: i32) -> i64;
+	def SYS_write(fd: i32, buf: Pointer, count: i32) -> i64;
+	def SYS_mmap(addr: Pointer, len: i32, prot: i32, flags: i32, fd: i32, offset: i64) -> i64;
+}
+
+def STDOUT = 1;
+def O_RDONLY = 0;
+def PROT_READ = 0x1;
+def MAP_PRIVATE = 0x02;
+
+def main() -> i64 {
+	var filename = "lorem-ipsum.txt\x00";
+	var fd = wali.SYS_open(Pointer.atContents(filename), O_RDONLY, 0);
+	check("open", fd >= 0);
+
+	var addr = wali.SYS_mmap(Pointer.NULL, 16, PROT_READ, MAP_PRIVATE, i32.view(fd), 0);
+	check("mmap", addr >= 0);
+	wali.SYS_close(i32.view(fd));
+
+	// The returned value is a linear memory offset, so the file contents must be
+	// readable through an ordinary guest load.
+	var p = Pointer.NULL + int.!(addr);
+	var expect = "Lorem ipsum";
+	var ok = true;
+	for (i < expect.length) if ((p + i).load<byte>() != expect[i]) ok = false;
+	check("file contents visible in linear memory", ok);
+	return 0;
+}
+
+def check(name: string, cond: bool) {
+	print(name);
+	print(if(cond, ": ok\n", ": FAIL\n"));
+}
+
+def print(str: string) {
+	wali.SYS_write(STDOUT, Pointer.atContents(str), str.length);
+}

test/wali/MmapFile.wasm.exit

@@ -0,0 +1 @@
+0

test/wali/MmapFile.wasm.out

@@ -0,0 +1,3 @@
+open: ok
+mmap: ok
+file contents visible in linear memory: ok

test/wali/NullPtr.v3

@@ -0,0 +1,41 @@
+// A guest NULL passed for an optional pointer argument must reach the kernel as NULL,
+// not as the host address of guest offset 0. Otherwise "argument absent" silently
+// becomes "argument present, pointing at the start of linear memory", and the kernel
+// either reads garbage or scribbles on the guest.
+import component wali {
+	def SYS_write(fd: i32, buf: Pointer, count: i32) -> i64;
+	def SYS_prlimit64(pid: i32, resource: i32, new_limit: Pointer, old_limit: Pointer) -> i64;
+	def SYS_mprotect(addr: Pointer, len: i32, prot: i32) -> i64;
+}
+
+def STDOUT = 1;
+def RLIMIT_NOFILE = 7;
+def PROT_READ = 0x1;
+
+def main() -> i64 {
+	var rlim = Array<byte>.new(16); // struct rlimit64 { u64 rlim_cur; u64 rlim_max; }
+	// A NULL new_limit means "query only". If it were translated to guest offset 0 the
+	// kernel would instead try to *set* the limit from whatever bytes live there.
+	var r = wali.SYS_prlimit64(0, RLIMIT_NOFILE, Pointer.NULL, Pointer.atContents(rlim));
+	check("prlimit64 query with NULL new_limit", r == 0);
+	check("prlimit64 wrote a soft limit", nonzero(rlim, 0, 8));
+	check("prlimit64 wrote a hard limit", nonzero(rlim, 8, 8));
+	// A NULL address cannot be mprotected, so this must fail. If NULL were translated
+	// to the base of linear memory it would succeed instead.
+	check("mprotect NULL fails", wali.SYS_mprotect(Pointer.NULL, 4096, PROT_READ) < 0);
+	return 0;
+}
+
+def nonzero(a: Array<byte>, start: int, len: int) -> bool {
+	for (i < len) if (a[start + i] != 0) return true;
+	return false;
+}
+
+def check(name: string, cond: bool) {
+	print(name);
+	print(if(cond, ": ok\n", ": FAIL\n"));
+}
+
+def print(str: string) {
+	wali.SYS_write(STDOUT, Pointer.atContents(str), str.length);
+}

test/wali/NullPtr.wasm.exit

@@ -0,0 +1 @@
+0

test/wali/NullPtr.wasm.out

@@ -0,0 +1,4 @@
+prlimit64 query with NULL new_limit: ok
+prlimit64 wrote a soft limit: ok
+prlimit64 wrote a hard limit: ok
+mprotect NULL fails: ok

test/wali/SocketPair.v3

@@ -0,0 +1,38 @@
+// socketpair hands its two fds back through guest memory rather than as its result.
+// They must be registered in the engine's fd table and rewritten to guest fds; leaving
+// raw host fd numbers there makes every later use resolve to -1 and fail with EBADF.
+import component wali {
+	def SYS_write(fd: i32, buf: Pointer, count: i32) -> i64;
+	def SYS_read(fd: i32, buf: Pointer, count: i32) -> i64;
+	def SYS_close(fd: i32) -> i64;
+	def SYS_socketpair(domain: i32, stype: i32, protocol: i32, sv: Pointer) -> i64;
+}
+
+def STDOUT = 1;
+def AF_UNIX = 1;
+def SOCK_STREAM = 1;
+
+def main() -> i64 {
+	var sv = Array<int>.new(2);
+	check("socketpair", wali.SYS_socketpair(AF_UNIX, SOCK_STREAM, 0, Pointer.atContents(sv)) == 0);
+
+	var msg = "ping";
+	check("write to sv[0]", wali.SYS_write(sv[0], Pointer.atContents(msg), msg.length) == 4);
+
+	var buf = Array<byte>.new(4);
+	check("read from sv[1]", wali.SYS_read(sv[1], Pointer.atContents(buf), 4) == 4);
+	check("round trip", buf[0] == 'p' && buf[1] == 'i' && buf[2] == 'n' && buf[3] == 'g');
+
+	check("close sv[0]", wali.SYS_close(sv[0]) == 0);
+	check("close sv[1]", wali.SYS_close(sv[1]) == 0);
+	return 0;
+}
+
+def check(name: string, cond: bool) {
+	print(name);
+	print(if(cond, ": ok\n", ": FAIL\n"));
+}
+
+def print(str: string) {
+	wali.SYS_write(STDOUT, Pointer.atContents(str), str.length);
+}

test/wali/SocketPair.wasm.exit

@@ -0,0 +1 @@
+0

test/wali/SocketPair.wasm.out

@@ -0,0 +1,6 @@
+socketpair: ok
+write to sv[0]: ok
+read from sv[1]: ok
+round trip: ok
+close sv[0]: ok
+close sv[1]: ok