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: https://github.com/nodejs/node/pull/16871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Ben Noordhuis 2017-11-07 21:28:13 +01:00
parent e5238ed030
commit f823d381e7
3 changed files with 21 additions and 6 deletions

View File

@ -515,12 +515,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

1
test/fixtures/empty-with-bom.txt vendored Normal file
View File

@ -0,0 +1 @@


View File

@ -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')), '');