From 8e76397fab69f79dfd77147360e20e004b65beed Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Fri, 9 Apr 2021 21:39:43 -0400
Subject: [PATCH] fs: validate encoding to binding.writeString()

The binding layer performs some validation of the encoding and
data passed to WriteString(). This commit adds similar validation
to the JS layer for better error handling.

PR-URL: https://github.com/nodejs/node/pull/38183
Fixes: https://github.com/nodejs/node/issues/38168
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
---
 lib/fs.js                         |  7 ++++++-
 lib/internal/fs/promises.js       |  2 ++
 test/parallel/test-fs-promises.js | 16 ++++++++++++++++
 test/parallel/test-fs-write.js    | 29 +++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/lib/fs.js b/lib/fs.js
index 6bf068416f1d..937b45cdb549 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -133,6 +133,7 @@ const {
   validateBoolean,
   validateBuffer,
   validateCallback,
+  validateEncoding,
   validateFunction,
   validateInteger,
 } = require('internal/validators');
@@ -702,11 +703,14 @@ function write(fd, buffer, offset, length, position, callback) {
     }
     length = 'utf8';
   }
+
+  const str = String(buffer);
+  validateEncoding(str, length);
   callback = maybeCallback(position);
 
   const req = new FSReqCallback();
   req.oncomplete = wrapper;
-  return binding.writeString(fd, String(buffer), offset, length, req);
+  return binding.writeString(fd, str, offset, length, req);
 }
 
 ObjectDefineProperty(write, internalUtil.customPromisifyArgs,
@@ -735,6 +739,7 @@ function writeSync(fd, buffer, offset, length, position) {
                                  undefined, ctx);
   } else {
     validateStringAfterArrayBufferView(buffer, 'buffer');
+    validateEncoding(buffer, length);
 
     if (offset === undefined)
       offset = null;
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
index 791bf57a7d70..683969e2ec2b 100644
--- a/lib/internal/fs/promises.js
+++ b/lib/internal/fs/promises.js
@@ -70,6 +70,7 @@ const {
   validateAbortSignal,
   validateBoolean,
   validateBuffer,
+  validateEncoding,
   validateInteger,
 } = require('internal/validators');
 const pathModule = require('path');
@@ -467,6 +468,7 @@ async function write(handle, buffer, offset, length, position) {
   }
 
   validateStringAfterArrayBufferView(buffer, 'buffer');
+  validateEncoding(buffer, length);
   const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
                                                   length, kUsePromises)) || 0;
   return { bytesWritten, buffer };
diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js
index 35d2deff8b81..df7fa3e4733c 100644
--- a/test/parallel/test-fs-promises.js
+++ b/test/parallel/test-fs-promises.js
@@ -436,6 +436,22 @@ async function getHandle(dest) {
       );
     }
 
+    // Regression test for https://github.com/nodejs/node/issues/38168
+    {
+      const handle = await getHandle(dest);
+
+      assert.rejects(
+        async () => handle.write('abc', 0, 'hex'),
+        {
+          code: 'ERR_INVALID_ARG_VALUE',
+          message: /'encoding' is invalid for data of length 3/
+        }
+      );
+
+      const ret = await handle.write('abcd', 0, 'hex');
+      assert.strictEqual(ret.bytesWritten, 2);
+      await handle.close();
+    }
   }
 
   doTest().then(common.mustCall());
diff --git a/test/parallel/test-fs-write.js b/test/parallel/test-fs-write.js
index 63f0245663c4..30e25b15808d 100644
--- a/test/parallel/test-fs-write.js
+++ b/test/parallel/test-fs-write.js
@@ -33,6 +33,7 @@ const fn = path.join(tmpdir.path, 'write.txt');
 const fn2 = path.join(tmpdir.path, 'write2.txt');
 const fn3 = path.join(tmpdir.path, 'write3.txt');
 const fn4 = path.join(tmpdir.path, 'write4.txt');
+const fn5 = path.join(tmpdir.path, 'write5.txt');
 const expected = 'ümlaut.';
 const constants = fs.constants;
 
@@ -170,3 +171,31 @@ fs.open(fn4, 'w', 0o644, common.mustSucceed((fd) => {
     }
   );
 });
+
+{
+  // Regression test for https://github.com/nodejs/node/issues/38168
+  const fd = fs.openSync(fn5, 'w');
+
+  assert.throws(
+    () => fs.writeSync(fd, 'abc', 0, 'hex'),
+    {
+      code: 'ERR_INVALID_ARG_VALUE',
+      message: /'encoding' is invalid for data of length 3/
+    }
+  );
+
+  assert.throws(
+    () => fs.writeSync(fd, 'abc', 0, 'hex', common.mustNotCall()),
+    {
+      code: 'ERR_INVALID_ARG_VALUE',
+      message: /'encoding' is invalid for data of length 3/
+    }
+  );
+
+  assert.strictEqual(fs.writeSync(fd, 'abcd', 0, 'hex'), 2);
+
+  fs.write(fd, 'abcd', 0, 'hex', common.mustSucceed((written) => {
+    assert.strictEqual(written, 2);
+    fs.closeSync(fd);
+  }));
+}