src: fix UB in InternalModuleReadFile()

`&vec[0]` is undefined behavior when `vec.size() == 0`.

It is mostly academic because package.json files are not usually empty
and because with most STL implementations it decays to something that
is legal C++ as long as the result is not dereferenced, but better safe
than sorry.

Note that the tests don't actually fail because of that, I added them
as sanity checks.

PR-URL: #16871
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>

src/node_file.cc

@@ -539,12 +539,17 @@ static void InternalModuleReadFile(const FunctionCallbackInfo<Value>& args) {
     start = 3;  // Skip UTF-8 BOM.
   }
 
-  Local<String> chars_string =
-      String::NewFromUtf8(env->isolate(),
-                          &chars[start],
-                          String::kNormalString,
-                          offset - start);
-  args.GetReturnValue().Set(chars_string);
+  const size_t size = offset - start;
+  if (size == 0) {
+    args.GetReturnValue().SetEmptyString();
+  } else {
+    Local<String> chars_string =
+        String::NewFromUtf8(env->isolate(),
+                            &chars[start],
+                            String::kNormalString,
+                            size);
+    args.GetReturnValue().Set(chars_string);
+  }
 }
 
 // Used to speed up module loading.  Returns 0 if the path refers to

test/fixtures/empty-with-bom.txt

@@ -0,0 +1 @@
+

test/parallel/test-module-binding.js

@@ -0,0 +1,9 @@
+'use strict';
+require('../common');
+const fixtures = require('../common/fixtures');
+const { internalModuleReadFile } = process.binding('fs');
+const { strictEqual } = require('assert');
+
+strictEqual(internalModuleReadFile('nosuchfile'), undefined);
+strictEqual(internalModuleReadFile(fixtures.path('empty.txt')), '');
+strictEqual(internalModuleReadFile(fixtures.path('empty-with-bom.txt')), '');