src: Check for overflow when resizing AliasedBufferBase

Makefile

@@ -292,7 +292,7 @@ v8:
 	tools/make-v8.sh $(V8_ARCH).$(BUILDTYPE_LOWER) $(V8_BUILD_OPTIONS)
 
 .PHONY: jstest
-jstest: build-addons build-js-native-api-tests build-node-api-tests ## Runs addon tests and JS tests
+jstest: build-addons build-abort-tests build-js-native-api-tests build-node-api-tests ## Runs addon tests and JS tests
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) \
 		--skip-tests=$(CI_SKIP_TESTS) \
 		$(CI_JS_SUITES) \
@@ -316,6 +316,7 @@ test: all ## Runs default tests, linters, and builds docs.
 	$(MAKE) -s tooltest
 	$(MAKE) -s test-doc
 	$(MAKE) -s build-addons
+	$(MAKE) -s build-abort-tests
 	$(MAKE) -s build-js-native-api-tests
 	$(MAKE) -s build-node-api-tests
 	$(MAKE) -s cctest
@@ -324,6 +325,7 @@ test: all ## Runs default tests, linters, and builds docs.
 .PHONY: test-only
 test-only: all  ## For a quick test, does not run linter or build docs.
 	$(MAKE) build-addons
+	$(MAKE) build-abort-tests
 	$(MAKE) build-js-native-api-tests
 	$(MAKE) build-node-api-tests
 	$(MAKE) cctest
@@ -333,6 +335,7 @@ test-only: all  ## For a quick test, does not run linter or build docs.
 # Used by `make coverage-test`
 test-cov: all
 	$(MAKE) build-addons
+	$(MAKE) build-abort-tests
 	$(MAKE) build-js-native-api-tests
 	$(MAKE) build-node-api-tests
 	$(MAKE) cctest
@@ -452,6 +455,31 @@ test/node-api/.buildstamp: $(ADDONS_PREREQS) \
 # TODO(bnoordhuis) Force rebuild after gyp or node-gyp update.
 build-node-api-tests: | $(NODE_EXE) test/node-api/.buildstamp
 
+ABORT_BINDING_GYPS := \
+	$(filter-out test/abort/??_*/binding.gyp, \
+		$(wildcard test/abort/*/binding.gyp))
+
+ABORT_BINDING_SOURCES := \
+	$(filter-out test/abort/??_*/*.c, $(wildcard test/abort/*/*.c)) \
+	$(filter-out test/abort/??_*/*.cc, $(wildcard test/abort/*/*.cc)) \
+	$(filter-out test/abort/??_*/*.h, $(wildcard test/abort/*/*.h))
+
+# Implicitly depends on $(NODE_EXE), see the build-node-api-tests rule for rationale.
+test/abort/.buildstamp: $(ADDONS_PREREQS) \
+	$(ABORT_BINDING_GYPS) $(ABORT_BINDING_SOURCES) \
+	src/node_api.h src/node_api_types.h src/js_native_api.h \
+	src/js_native_api_types.h src/js_native_api_v8.h src/js_native_api_v8_internals.h
+	@$(call run_build_addons,"$$PWD/test/abort",$@)
+
+.PHONY: build-abort-tests
+# .buildstamp needs $(NODE_EXE) but cannot depend on it
+# directly because it calls make recursively.  The parent make cannot know
+# if the subprocess touched anything so it pessimistically assumes that
+# .buildstamp is out of date and need a rebuild.
+# Just goes to show that recursive make really is harmful...
+# TODO(bnoordhuis) Force rebuild after gyp or node-gyp update.
+build-abort-tests: | $(NODE_EXE) test/abort/.buildstamp
+
 BENCHMARK_NAPI_BINDING_GYPS := $(wildcard benchmark/napi/*/binding.gyp)
 
 BENCHMARK_NAPI_BINDING_SOURCES := \
@@ -472,12 +500,14 @@ clear-stalled:
 		echo $${PS_OUT} | xargs kill -9; \
 	fi
 
-test-build: | all build-addons build-js-native-api-tests build-node-api-tests
+test-build: | all build-addons build-abort-tests build-js-native-api-tests build-node-api-tests
 
 test-build-js-native-api: all build-js-native-api-tests
 
 test-build-node-api: all build-node-api-tests
 
+test-build-abort: all build-abort-tests
+
 .PHONY: test-all
 test-all: test-build ## Run default tests with both Debug and Release builds.
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=debug,release
@@ -490,7 +520,7 @@ test-all-suites: | clear-stalled test-build bench-addons-build doc-only ## Run a
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) test/*
 
 # CI_* variables should be kept synchronized with the ones in vcbuild.bat
-CI_NATIVE_SUITES ?= addons js-native-api node-api
+CI_NATIVE_SUITES ?= addons js-native-api node-api abort
 CI_JS_SUITES ?= default
 ifeq ($(node_use_openssl), false)
 	CI_DOC := doctool
@@ -502,7 +532,7 @@ endif
 # Build and test addons without building anything else
 # Related CI job: node-test-commit-arm-fanned
 test-ci-native: LOGLEVEL := info
-test-ci-native: | test/addons/.buildstamp test/js-native-api/.buildstamp test/node-api/.buildstamp
+test-ci-native: | test/addons/.buildstamp test/js-native-api/.buildstamp test/node-api/.buildstamp test/abort/.buildstamp
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \
 		--mode=$(BUILDTYPE_LOWER) --flaky-tests=$(FLAKY_TESTS) \
 		$(TEST_CI_ARGS) $(CI_NATIVE_SUITES)
@@ -524,7 +554,7 @@ test-ci-js: | clear-stalled
 .PHONY: test-ci
 # Related CI jobs: most CI tests, excluding node-test-commit-arm-fanned
 test-ci: LOGLEVEL := info
-test-ci: | clear-stalled build-addons build-js-native-api-tests build-node-api-tests doc-only
+test-ci: | clear-stalled build-addons build-abort-tests build-js-native-api-tests build-node-api-tests doc-only
 	out/Release/cctest --gtest_output=xml:out/junit/cctest.xml
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \
 		--mode=$(BUILDTYPE_LOWER) --flaky-tests=$(FLAKY_TESTS) \
@@ -628,8 +658,17 @@ test-node-api-clean:
 	$(RM) -r test/node-api/*/build
 	$(RM) test/node-api/.buildstamp
 
+.PHONY: test-abort
+test-abort: test-build-abort
+	$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) test-abort
+
+.PHONY: test-abort-clean
+test-abort-clean:
+	$(RM) -r test/abort/*/build
+	$(RM) test/abort/.buildstamp
+
 .PHONY: test-addons
-test-addons: test-build test-js-native-api test-node-api
+test-addons: test-build test-js-native-api test-node-api test-abort
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) addons
 
 .PHONY: test-addons-clean
@@ -639,6 +678,7 @@ test-addons-clean:
 	$(RM) test/addons/.buildstamp test/addons/.docbuildstamp
 	$(MAKE) test-js-native-api-clean
 	$(MAKE) test-node-api-clean
+	$(MAKE) test-abort-clean
 
 test-async-hooks:
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) async-hooks
@@ -647,6 +687,7 @@ test-with-async-hooks:
 	$(MAKE) build-addons
 	$(MAKE) build-js-native-api-tests
 	$(MAKE) build-node-api-tests
+	$(MAKE) build-abort-tests
 	$(MAKE) cctest
 	NODE_TEST_WITH_ASYNC_HOOKS=1 $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) \
 		$(CI_JS_SUITES) \

src/aliased_buffer.h

@@ -221,7 +221,8 @@ class AliasedBufferBase {
     const v8::HandleScope handle_scope(isolate_);
 
     const size_t old_size_in_bytes = sizeof(NativeT) * count_;
-    const size_t new_size_in_bytes = sizeof(NativeT) * new_capacity;
+    const size_t new_size_in_bytes = MultiplyWithOverflowCheck(sizeof(NativeT),
+                                                              new_capacity);
 
     // allocate v8 new ArrayBuffer
     v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(

test/abort/common.gypi

@@ -0,0 +1,8 @@
+{
+  'defines': [ 'V8_DEPRECATION_WARNINGS=1', 'NODE_WANT_INTERNALS=1' ],
+  'conditions': [
+    [ 'OS in "linux freebsd openbsd solaris android aix cloudabi"', {
+      'cflags': ['-Wno-cast-function-type'],
+    }],
+  ],
+}

test/abort/test_abort-aliased-buffer-overflow/binding.cc

@@ -0,0 +1,23 @@
+#include <stdlib.h>
+#include <node.h>
+#include <v8.h>
+
+#include <aliased_buffer.h>
+#include <util-inl.h>
+
+void AllocateAndResizeBuffer(
+  const v8::FunctionCallbackInfo<v8::Value>& args) {
+    v8::Isolate* isolate = args.GetIsolate();
+    int64_t length = args[0].As<v8::BigInt>()->Int64Value();
+
+     node::AliasedBigUint64Array array{isolate, 0};
+
+    array.reserve(length);
+    assert(false);
+  }
+
+void init(v8::Local<v8::Object> exports) {
+  NODE_SET_METHOD(exports,
+                  "allocateAndResizeBuffer",
+                  AllocateAndResizeBuffer);
+}

test/abort/test_abort-aliased-buffer-overflow/binding.gyp

@@ -0,0 +1,9 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'sources': [ 'binding.cc' ],
+      'includes': ['../common.gypi'],
+    }
+  ]
+}

test/abort/test_abort-aliased-buffer-overflow/test-abort-aliased-buffer-overflow.js

@@ -0,0 +1,28 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+
+// This test ensures that during resizing of an Aliased*Array the computation
+// of the new size does not overflow.
+
+if (process.argv[2] === 'child') {
+  // test
+  const binding = require(`./build/${common.buildType}/binding`);
+
+  const bigValue = BigInt('0xE000 0000 E000 0000');
+  binding.AllocateAndResizeBuffer(bigValue);
+  assert.fail('this should be unreachable');
+} else {
+  // observer
+  const child = cp.spawn(`${process.execPath}`, [`${__filename}`, 'child']);
+  child.on('exit', common.mustCall(function(code, signal) {
+    if (common.isWindows) {
+      assert.strictEqual(code, 134);
+      assert.strictEqual(signal, null);
+    } else {
+      assert.strictEqual(code, null);
+      assert.strictEqual(signal, 'SIGABRT');
+    }
+  }));
+}

vcbuild.bat

@@ -16,11 +16,11 @@ if /i "%1"=="/?" goto help
 cd %~dp0
 
 @rem CI_* variables should be kept synchronized with the ones in Makefile
-set CI_NATIVE_SUITES=addons js-native-api node-api
+set CI_NATIVE_SUITES=addons js-native-api node-api abort
 set CI_JS_SUITES=default
 set CI_DOC=doctool
 @rem Same as the test-ci target in Makefile
-set "common_test_suites=%CI_JS_SUITES% %CI_NATIVE_SUITES% %CI_DOC%&set build_addons=1&set build_js_native_api_tests=1&set build_node_api_tests=1"
+set "common_test_suites=%CI_JS_SUITES% %CI_NATIVE_SUITES% %CI_DOC%&set build_addons=1&set build_js_native_api_tests=1&set build_node_api_tests=1&set build_aborts_tests=1"
 
 @rem Process arguments.
 set config=Release
@@ -68,6 +68,7 @@ set openssl_no_asm=
 set doc=
 set extra_msbuild_args=
 set exit_code=0
+set build_aborts_tests=
 
 :next-arg
 if "%1"=="" goto args-done
@@ -96,6 +97,8 @@ if /i "%1"=="test-ci-js"    set test_args=%test_args% %test_ci_args% -J -p tap -
 if /i "%1"=="build-addons"   set build_addons=1&goto arg-ok
 if /i "%1"=="build-js-native-api-tests"   set build_js_native_api_tests=1&goto arg-ok
 if /i "%1"=="build-node-api-tests"   set build_node_api_tests=1&goto arg-ok
+if /i "%1"=="build-abort-tests"   set build_abort_tests=1&goto arg-ok
+if /i "%1"=="test-abort"   set test_args=%test_args% abort&set build_abort_tests=1&goto arg-ok
 if /i "%1"=="test-addons"   set test_args=%test_args% addons&set build_addons=1&goto arg-ok
 if /i "%1"=="test-js-native-api"   set test_args=%test_args% js-native-api&set build_js_native_api_tests=1&goto arg-ok
 if /i "%1"=="test-node-api"   set test_args=%test_args% node-api&set build_node_api_tests=1&goto arg-ok
@@ -585,10 +588,10 @@ endlocal
 goto build-node-api-tests
 
 :build-node-api-tests
-if not defined build_node_api_tests goto run-tests
+if not defined build_node_api_tests goto build-abort-tests
 if not exist "%node_exe%" (
   echo Failed to find node.exe
-  goto run-tests
+  goto build-abort-tests
 )
 echo Building node-api
 :: clear
@@ -601,6 +604,25 @@ set npm_config_nodedir=%~dp0
 "%node_exe%" "%~dp0tools\build-addons.js" "%~dp0deps\npm\node_modules\node-gyp\bin\node-gyp.js" "%~dp0test\node-api"
 if errorlevel 1 exit /b 1
 endlocal
+goto build-abort-tests
+
+:build-abort-tests
+if not defined build_abort_tests goto run-tests
+if not exist "%node_exe%" (
+  echo Failed to find node.exe
+  goto run-tests
+)
+echo Building abort
+:: clear
+for /d %%F in (test\abort\??_*) do (
+  rd /s /q %%F
+)
+:: building abort
+setlocal
+set npm_config_nodedir=%~dp0
+"%node_exe%" "%~dp0tools\build-addons.js" "%~dp0deps\npm\node_modules\node-gyp\bin\node-gyp.js" "%~dp0test\abort"
+if errorlevel 1 exit /b 1
+endlocal
 goto run-tests
 
 :run-tests