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 6bf068416f1daa..937b45cdb549a3 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 791bf57a7d70b4..683969e2ec2b50 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 35d2deff8b8148..df7fa3e4733cba 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 63f0245663c435..30e25b15808dd2 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);
+  }));
+}