From 378e0e778b0bf7e1d60797e64db42d989a1986e5 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Mon, 26 Apr 2021 10:58:41 -0700
Subject: [PATCH] src: fix validation of negative offset to avoid abort

Fixes: https://github.com/nodejs/node/issues/24640
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/38421
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
---
 lib/fs.js                                     |  8 +--
 lib/internal/fs/promises.js                   |  4 +-
 lib/internal/fs/utils.js                      |  4 ++
 test/parallel/test-fs-read-type.js            |  4 --
 test/parallel/test-fs-write-negativeoffset.js | 55 +++++++++++++++++++
 5 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100644 test/parallel/test-fs-write-negativeoffset.js

diff --git a/lib/fs.js b/lib/fs.js
index a19f84b4f404bd..7325a24f080182 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -535,7 +535,7 @@ function read(fd, buffer, offset, length, position, callback) {
   if (offset == null) {
     offset = 0;
   } else {
-    validateInteger(offset, 'offset');
+    validateInteger(offset, 'offset', 0);
   }
 
   length |= 0;
@@ -589,7 +589,7 @@ function readSync(fd, buffer, offset, length, position) {
   if (offset == null) {
     offset = 0;
   } else {
-    validateInteger(offset, 'offset');
+    validateInteger(offset, 'offset', 0);
   }
 
   length |= 0;
@@ -667,7 +667,7 @@ function write(fd, buffer, offset, length, position, callback) {
     if (offset == null || typeof offset === 'function') {
       offset = 0;
     } else {
-      validateInteger(offset, 'offset');
+      validateInteger(offset, 'offset', 0);
     }
     if (typeof length !== 'number')
       length = buffer.length - offset;
@@ -715,7 +715,7 @@ function writeSync(fd, buffer, offset, length, position) {
     if (offset == null) {
       offset = 0;
     } else {
-      validateInteger(offset, 'offset');
+      validateInteger(offset, 'offset', 0);
     }
     if (typeof length !== 'number')
       length = buffer.byteLength - offset;
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
index 7b2c526b59850e..39418446bd9c15 100644
--- a/lib/internal/fs/promises.js
+++ b/lib/internal/fs/promises.js
@@ -366,7 +366,7 @@ async function read(handle, bufferOrOptions, offset, length, position) {
   if (offset == null) {
     offset = 0;
   } else {
-    validateInteger(offset, 'offset');
+    validateInteger(offset, 'offset', 0);
   }
 
   length |= 0;
@@ -409,7 +409,7 @@ async function write(handle, buffer, offset, length, position) {
     if (offset == null) {
       offset = 0;
     } else {
-      validateInteger(offset, 'offset');
+      validateInteger(offset, 'offset', 0);
     }
     if (typeof length !== 'number')
       length = buffer.length - offset;
diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
index b935c495c6883f..abc12b7b02a4ff 100644
--- a/lib/internal/fs/utils.js
+++ b/lib/internal/fs/utils.js
@@ -620,6 +620,10 @@ const validateOffsetLengthWrite = hideStackFrames(
     if (length > byteLength - offset) {
       throw new ERR_OUT_OF_RANGE('length', `<= ${byteLength - offset}`, length);
     }
+
+    if (length < 0) {
+      throw new ERR_OUT_OF_RANGE('length', '>= 0', length);
+    }
   }
 );
 
diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js
index 0f9bdbab588661..02298ed1c3523d 100644
--- a/test/parallel/test-fs-read-type.js
+++ b/test/parallel/test-fs-read-type.js
@@ -44,8 +44,6 @@ assert.throws(() => {
 }, {
   code: 'ERR_OUT_OF_RANGE',
   name: 'RangeError',
-  message: 'The value of "offset" is out of range. It must be >= 0. ' +
-           'Received -1'
 });
 
 assert.throws(() => {
@@ -109,8 +107,6 @@ assert.throws(() => {
 }, {
   code: 'ERR_OUT_OF_RANGE',
   name: 'RangeError',
-  message: 'The value of "offset" is out of range. ' +
-           'It must be >= 0. Received -1'
 });
 
 assert.throws(() => {
diff --git a/test/parallel/test-fs-write-negativeoffset.js b/test/parallel/test-fs-write-negativeoffset.js
new file mode 100644
index 00000000000000..b1c6ed9039b7d7
--- /dev/null
+++ b/test/parallel/test-fs-write-negativeoffset.js
@@ -0,0 +1,55 @@
+'use strict';
+
+// Tests that passing a negative offset does not crash the process
+
+const common = require('../common');
+
+const {
+  join,
+} = require('path');
+
+const {
+  closeSync,
+  open,
+  write,
+  writeSync,
+} = require('fs');
+
+const assert = require('assert');
+
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+
+const filename = join(tmpdir.path, 'test.txt');
+
+open(filename, 'w+', common.mustSucceed((fd) => {
+  assert.throws(() => {
+    write(fd, Buffer.alloc(0), -1, common.mustNotCall());
+  }, {
+    code: 'ERR_OUT_OF_RANGE',
+  });
+  assert.throws(() => {
+    writeSync(fd, Buffer.alloc(0), -1);
+  }, {
+    code: 'ERR_OUT_OF_RANGE',
+  });
+  closeSync(fd);
+}));
+
+const filename2 = join(tmpdir.path, 'test2.txt');
+
+// Make sure negative length's don't cause aborts either
+
+open(filename2, 'w+', common.mustSucceed((fd) => {
+  assert.throws(() => {
+    write(fd, Buffer.alloc(0), 0, -1, common.mustNotCall());
+  }, {
+    code: 'ERR_OUT_OF_RANGE',
+  });
+  assert.throws(() => {
+    writeSync(fd, Buffer.alloc(0), 0, -1);
+  }, {
+    code: 'ERR_OUT_OF_RANGE',
+  });
+  closeSync(fd);
+}));