Fix use-after-free in filter() / map() plus some other stuff

lib.c

@@ -934,6 +934,7 @@ uc_filter(uc_vm_t *vm, size_t nargs)
 		return NULL;
 
 	arr = ucv_array_new(vm);
+	uc_vm_stack_push(vm, ucv_get(arr));
 
 	for (arrlen = ucv_array_length(obj), arridx = 0; arridx < arrlen; arridx++) {
 		uc_vm_ctx_push(vm);
@@ -956,6 +957,8 @@ uc_filter(uc_vm_t *vm, size_t nargs)
 		ucv_put(rv);
 	}
 
+	ucv_put(uc_vm_stack_pop(vm));
+
 	return arr;
 }
 
@@ -1224,6 +1227,7 @@ uc_map(uc_vm_t *vm, size_t nargs)
 		return NULL;
 
 	arr = ucv_array_new(vm);
+	uc_vm_stack_push(vm, ucv_get(arr));
 
 	for (arrlen = ucv_array_length(obj), arridx = 0; arridx < arrlen; arridx++) {
 		uc_vm_ctx_push(vm);
@@ -1243,6 +1247,8 @@ uc_map(uc_vm_t *vm, size_t nargs)
 		ucv_array_push(arr, rv);
 	}
 
+	ucv_put(uc_vm_stack_pop(vm));
+
 	return arr;
 }
 
@@ -3463,17 +3469,17 @@ uc_json_from_object(uc_vm_t *vm, uc_value_t *obj, json_object **jso)
 {
 	bool trail = false, eof = false;
 	enum json_tokener_error err;
-	struct json_tokener *tok;
-	uc_value_t *rfn, *rbuf;
+	struct json_tokener *tok = NULL;
+	uc_value_t *rfn, *rbuf = NULL;
 	uc_stringbuf_t *buf;
 
-	rfn = ucv_property_get(obj, "read");
+	rfn = ucv_get(ucv_property_get(obj, "read"));
 
 	if (!ucv_is_callable(rfn)) {
 		uc_vm_raise_exception(vm, EXCEPTION_TYPE,
 		                      "Input object does not implement read() method");
 
-		return NULL;
+		goto out;
 	}
 
 	tok = xjs_new_tokener();
@@ -3483,11 +3489,8 @@ uc_json_from_object(uc_vm_t *vm, uc_value_t *obj, json_object **jso)
 		uc_vm_stack_push(vm, ucv_get(rfn));
 		uc_vm_stack_push(vm, ucv_int64_new(1024));
 
-		if (uc_vm_call(vm, true, 1) != EXCEPTION_NONE) {
-			json_tokener_free(tok);
-
-			return NULL;
-		}
+		if (uc_vm_call(vm, true, 1) != EXCEPTION_NONE)
+			goto fail;
 
 		rbuf = uc_vm_stack_pop(vm);
 
@@ -3496,8 +3499,6 @@ uc_json_from_object(uc_vm_t *vm, uc_value_t *obj, json_object **jso)
 
 		/* on EOF, stop parsing unless trailing garbage was detected which handled below */
 		if (eof && !trail) {
-			ucv_put(rbuf);
-
 			/* Didn't parse a complete object yet, possibly a non-delimited atomic value
 			   such as `null`, `true` etc. - nudge parser by sending final zero byte.
 			   See json-c issue #681 <https://github.com/json-c/json-c/issues/681> */
@@ -3511,10 +3512,7 @@ uc_json_from_object(uc_vm_t *vm, uc_value_t *obj, json_object **jso)
 			uc_vm_raise_exception(vm, EXCEPTION_SYNTAX,
 			                      "Trailing garbage after JSON data");
 
-			json_tokener_free(tok);
-			ucv_put(rbuf);
-
-			return NULL;
+			goto fail;
 		}
 
 		if (ucv_type(rbuf) != UC_STRING) {
@@ -3536,13 +3534,24 @@ uc_json_from_object(uc_vm_t *vm, uc_value_t *obj, json_object **jso)
 		}
 
 		ucv_put(rbuf);
+		rbuf = NULL;
 
 		err = json_tokener_get_error(tok);
 
 		if (err != json_tokener_success && err != json_tokener_continue)
 			break;
 	}
 
+	goto out;
+
+fail:
+	json_tokener_free(tok);
+	tok = NULL;
+
+out:
+	ucv_put(rfn);
+	ucv_put(rbuf);
+
 	return tok;
 }
 
@@ -5738,7 +5747,7 @@ static uc_value_t *
 uc_callfunc(uc_vm_t *vm, size_t nargs)
 {
 	size_t argoff = vm->stack.count - nargs, i;
-	uc_value_t *fn_scope, *prev_scope, *res;
+	uc_value_t *fn_scope, *prev_scope = NULL, *res;
 	uc_value_t *fn = uc_fn_arg(0);
 	uc_value_t *this = uc_fn_arg(1);
 	uc_value_t *scope = uc_fn_arg(2);
@@ -5764,24 +5773,27 @@ uc_callfunc(uc_vm_t *vm, size_t nargs)
 		fn_scope = NULL;
 	}
 
+	if (fn_scope) {
+		prev_scope = ucv_get(uc_vm_scope_get(vm));
+		uc_vm_stack_push(vm, ucv_get(prev_scope));
+		uc_vm_scope_set(vm, fn_scope);
+	}
+
 	uc_vm_stack_push(vm, ucv_get(this));
 	uc_vm_stack_push(vm, ucv_get(fn));
 
 	for (i = 3; i < nargs; i++)
 		uc_vm_stack_push(vm, ucv_get(vm->stack.entries[3 + argoff++]));
 
-	if (fn_scope) {
-		prev_scope = ucv_get(uc_vm_scope_get(vm));
-		uc_vm_scope_set(vm, fn_scope);
-	}
-
 	if (uc_vm_call(vm, true, i - 3) == EXCEPTION_NONE)
 		res = uc_vm_stack_pop(vm);
 	else
 		res = NULL;
 
-	if (fn_scope)
+	if (fn_scope) {
 		uc_vm_scope_set(vm, prev_scope);
+		ucv_put(uc_vm_stack_pop(vm));
+	}
 
 	return res;
 }

lib/socket.c

@@ -2701,11 +2701,14 @@ uc_socket_poll(uc_vm_t *vm, size_t nargs)
 	if (errno != 0 || timeout < (int64_t)INT_MIN || timeout > (int64_t)INT_MAX)
 		err_return(ERANGE, "Invalid timeout value");
 
+	size_t argoff = vm->stack.count - nargs;
+
 	rv = ucv_array_new(vm);
+	uc_vm_stack_push(vm, ucv_get(rv));
 
 	for (size_t i = 1; i < nargs; i++) {
 		uc_vector_grow(&pfds);
-		item = uv_to_pollfd(vm, uc_fn_arg(i), &pfds.entries[pfds.count]);
+		item = uv_to_pollfd(vm, vm->stack.entries[argoff + i], &pfds.entries[pfds.count]);
 
 		if (item)
 			ucv_array_set(rv, pfds.count++, item);
@@ -2714,6 +2717,7 @@ uc_socket_poll(uc_vm_t *vm, size_t nargs)
 	ret = poll(pfds.entries, pfds.count, timeout);
 
 	if (ret == -1) {
+		ucv_put(uc_vm_stack_pop(vm));
 		ucv_put(rv);
 		uc_vector_clear(&pfds);
 		err_return(errno, "poll()");
@@ -2723,6 +2727,7 @@ uc_socket_poll(uc_vm_t *vm, size_t nargs)
 		ucv_array_set(ucv_array_get(rv, i), 1,
 			ucv_int64_new(pfds.entries[i].revents));
 
+	ucv_put(uc_vm_stack_pop(vm));
 	uc_vector_clear(&pfds);
 	ok_return(rv);
 }

lib/uloop.c

@@ -1770,7 +1770,7 @@ uc_uloop_task(uc_vm_t *vm, size_t nargs)
 	cbs = ucv_array_new(NULL);
 	ucv_array_set(cbs, 0, ucv_get(output_cb));
 	ucv_array_set(cbs, 1, ucv_get(input_cb));
-	ucv_resource_value_set(task->cb.obj, 1, ucv_get(cbs));
+	ucv_resource_value_set(task->cb.obj, 1, cbs);
 
 	ok_return(task->cb.obj);
 }

lib/zlib.c

@@ -94,7 +94,7 @@ def_chunks(zstrm_t * const zstrm)
 
 	/* run deflate() on input until output buffer not full */
 	do {
-		printbuf_memset(zstrm->outbuf, printbuf_length(zstrm->outbuf) + CHUNK - 1, 0, 1);
+		printbuf_memset(zstrm->outbuf, -1, 0, CHUNK);
 		zstrm->outbuf->bpos -= CHUNK;
 
 		zstrm->strm.avail_out = CHUNK;
@@ -114,15 +114,15 @@ static bool
 uc_zlib_def_object(uc_vm_t *const vm, uc_value_t * const obj, zstrm_t * const zstrm)
 {
 	int ret;
-	bool eof = false;
-	uc_value_t *rfn, *rbuf;
+	bool eof = false, rv = false;
+	uc_value_t *rfn, *rbuf = NULL;
 
-	rfn = ucv_property_get(obj, "read");
+	rfn = ucv_get(ucv_property_get(obj, "read"));
 
 	if (!ucv_is_callable(rfn)) {
 		uc_vm_raise_exception(vm, EXCEPTION_TYPE,
 				      "Input object does not implement read() method");
-		return false;
+		goto out;
 	}
 
 	do {
@@ -132,15 +132,15 @@ uc_zlib_def_object(uc_vm_t *const vm, uc_value_t * const obj, zstrm_t * const zs
 		uc_vm_stack_push(vm, ucv_int64_new(CHUNK));
 
 		if (uc_vm_call(vm, true, 1) != EXCEPTION_NONE)
-			goto fail;
+			goto out;
 
 		rbuf = uc_vm_stack_pop(vm);	// read output chunk
 
 		/* we only accept strings */
 		if (rbuf != NULL && ucv_type(rbuf) != UC_STRING) {
 			uc_vm_raise_exception(vm, EXCEPTION_TYPE,
 					      "Input object read() method returned non-string value");
-			goto fail;
+			goto out;
 		}
 
 		/* check EOF */
@@ -154,14 +154,16 @@ uc_zlib_def_object(uc_vm_t *const vm, uc_value_t * const obj, zstrm_t * const zs
 		(void)ret;	// XXX make annoying compiler that ignores assert() happy
 
 		ucv_put(rbuf);	// release rbuf
+		rbuf = NULL;
 	} while (!eof);	// finish compression if all of source has been read in
 	assert(ret == Z_STREAM_END);	// stream will be complete
 
-	return true;
+	rv = true;
 
-fail:
+out:
 	ucv_put(rbuf);
-	return false;
+	ucv_put(rfn);
+	return rv;
 }
 
 static bool
@@ -297,7 +299,7 @@ inf_chunks(zstrm_t * const zstrm)
 
 	/* run inflate() on input until output buffer not full */
 	do {
-		printbuf_memset(zstrm->outbuf, printbuf_length(zstrm->outbuf) + CHUNK - 1, 0, 1);
+		printbuf_memset(zstrm->outbuf, -1, 0, CHUNK);
 		zstrm->outbuf->bpos -= CHUNK;
 
 		zstrm->strm.avail_out = CHUNK;
@@ -322,15 +324,15 @@ static bool
 uc_zlib_inf_object(uc_vm_t *const vm, uc_value_t * const obj, zstrm_t * const zstrm)
 {
 	int ret = Z_STREAM_ERROR;	// error out if EOF on first loop
-	bool eof = false;
-	uc_value_t *rfn, *rbuf;
+	bool eof = false, rv = false;
+	uc_value_t *rfn, *rbuf = NULL;
 
-	rfn = ucv_property_get(obj, "read");
+	rfn = ucv_get(ucv_property_get(obj, "read"));
 
 	if (!ucv_is_callable(rfn)) {
 		uc_vm_raise_exception(vm, EXCEPTION_TYPE,
 				      "Input object does not implement read() method");
-		return false;
+		goto out;
 	}
 
 	do {
@@ -340,15 +342,15 @@ uc_zlib_inf_object(uc_vm_t *const vm, uc_value_t * const obj, zstrm_t * const zs
 		uc_vm_stack_push(vm, ucv_int64_new(CHUNK));
 
 		if (uc_vm_call(vm, true, 1) != EXCEPTION_NONE)
-			goto fail;
+			goto out;
 
 		rbuf = uc_vm_stack_pop(vm);	// read output chunk
 
 		/* we only accept strings */
 		if (rbuf != NULL && ucv_type(rbuf) != UC_STRING) {
 			uc_vm_raise_exception(vm, EXCEPTION_TYPE,
 					      "Input object read() method returned non-string value");
-			goto fail;
+			goto out;
 		}
 
 		/* check EOF */
@@ -364,20 +366,19 @@ uc_zlib_inf_object(uc_vm_t *const vm, uc_value_t * const obj, zstrm_t * const zs
 		case Z_NEED_DICT:
 		case Z_DATA_ERROR:
 		case Z_MEM_ERROR:
-			goto fail;
+			goto out;
 		}
 
 		ucv_put(rbuf);	// release rbuf
+		rbuf = NULL;
 	} while (ret != Z_STREAM_END);	// done when inflate() says it's done
 
-	if (ret != Z_STREAM_END)	// data error
-		return false;
+	rv = (ret == Z_STREAM_END);	// data error otherwise
 
-	return true;
-
-fail:
+out:
 	ucv_put(rbuf);
-	return false;
+	ucv_put(rfn);
+	return rv;
 }
 
 static bool

tests/custom/99_bugs/53_filter_map_gc_use_after_free

@@ -0,0 +1,56 @@
+Test that `filter()`, `map()`, and `call()` survive a `gc("collect")`
+triggered from inside the user callback. The builtins hold their working
+values in C locals while user code runs; under the bug, a GC sweep frees
+them and the builtins return corrupted or dangling data.
+
+-- Testcase --
+{%
+	let input = [];
+	for (let i = 0; i < 10; i++)
+		push(input, i);
+
+	function clobber() {
+		gc("collect");
+
+		let trash = [];
+		for (let i = 0; i < 256; i++)
+			push(trash, sprintf("clobber_%04d", i));
+	}
+
+	let f = filter(input, function(x) { clobber(); return true; });
+	printf("filter: type=%s len=%d match=%s\n",
+		type(f), length(f), (f == null) ? "n/a" : (`${f}` == `${input}` ? "yes" : "no"));
+
+	let m = map(input, function(x) { clobber(); return x; });
+	printf("map: type=%s len=%d match=%s\n",
+		type(m), length(m), (m == null) ? "n/a" : (`${m}` == `${input}` ? "yes" : "no"));
+
+	// uc_callfunc prev_scope UAF: a scope with its own prototype takes the
+	// branch where fn_scope = ucv_get(scope), with no prototype linkage to
+	// the prior globals. The prior globals (`prev_scope`) is then held only
+	// in a C local while user code runs.
+	global.tag = "original";
+
+	// capture builtins as closures so the user code can run them after
+	// vm->globals has been swapped to the prototype-only sandbox scope
+	let mygc = gc, mypush = push, mysprintf = sprintf;
+
+	call(function() {
+		mygc("collect");
+		let trash = [];
+		for (let i = 0; i < 256; i++)
+			mypush(trash, mysprintf("clobber_%04d", i));
+	}, null, proto({}, {}));
+
+	// after call() returns, globals must have been restored to a live
+	// object. accessing global.tag goes through vm->globals; if it is
+	// dangling the lookup fails or returns garbage.
+	printf("call: globals.tag=%s\n", global.tag);
+%}
+-- End --
+
+-- Expect stdout --
+filter: type=array len=10 match=yes
+map: type=array len=10 match=yes
+call: globals.tag=original
+-- End --