Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 81970f8

Browse filesBrowse files
bnoordhuisgibfahn
authored andcommitted
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 <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>
1 parent 4ede5ec commit 81970f8
Copy full SHA for 81970f8

File tree

Expand file treeCollapse file tree

3 files changed

+21
-6
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+21
-6
lines changed
Open diff view settings
Collapse file

‎src/node_file.cc‎

Copy file name to clipboardExpand all lines: src/node_file.cc
+11-6Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -539,12 +539,17 @@ static void InternalModuleReadFile(const FunctionCallbackInfo<Value>& args) {
539539
start = 3; // Skip UTF-8 BOM.
540540
}
541541

542-
Local<String> chars_string =
543-
String::NewFromUtf8(env->isolate(),
544-
&chars[start],
545-
String::kNormalString,
546-
offset - start);
547-
args.GetReturnValue().Set(chars_string);
542+
const size_t size = offset - start;
543+
if (size == 0) {
544+
args.GetReturnValue().SetEmptyString();
545+
} else {
546+
Local<String> chars_string =
547+
String::NewFromUtf8(env->isolate(),
548+
&chars[start],
549+
String::kNormalString,
550+
size);
551+
args.GetReturnValue().Set(chars_string);
552+
}
548553
}
549554

550555
// Used to speed up module loading. Returns 0 if the path refers to
Collapse file

‎test/fixtures/empty-with-bom.txt‎

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+

Collapse file
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
'use strict';
2+
require('../common');
3+
const fixtures = require('../common/fixtures');
4+
const { internalModuleReadFile } = process.binding('fs');
5+
const { strictEqual } = require('assert');
6+
7+
strictEqual(internalModuleReadFile('nosuchfile'), undefined);
8+
strictEqual(internalModuleReadFile(fixtures.path('empty.txt')), '');
9+
strictEqual(internalModuleReadFile(fixtures.path('empty-with-bom.txt')), '');

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.