Skip to content

Commit e07adb3

Browse files
RafaelGSSaduh95
authored andcommitted
src: cleanup uv_fs_req before uv_fs_stat on existSync
Refs: https://hackerone.com/reports/3184178 Calling uv_fs_stat() without first calling uv_fs_req_cleanup() overwrites the pointer to the previously allocated buffer leading to a memory leak on windows PR-URL: #58915 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Stefan Stojanovic <[email protected]>
1 parent 02031a9 commit e07adb3

File tree

2 files changed

+48
-0
lines changed

2 files changed

+48
-0
lines changed

src/node_file.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,6 +1033,7 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
10331033
// will **not** return an error and is therefore not enough.
10341034
// Double check with `uv_fs_stat()`.
10351035
if (err == 0) {
1036+
uv_fs_req_cleanup(&req);
10361037
FS_SYNC_TRACE_BEGIN(stat);
10371038
err = uv_fs_stat(nullptr, &req, path.out(), nullptr);
10381039
FS_SYNC_TRACE_END(stat);
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Flags: --expose-gc --expose-internals
2+
'use strict';
3+
4+
const common = require('../common');
5+
const { checkIfCollectableByCounting } = require('../common/gc');
6+
const assert = require('assert');
7+
const fs = require('node:fs');
8+
const path = require('node:path');
9+
const tmpdir = require('../common/tmpdir');
10+
const { internalBinding } = require('internal/test/binding');
11+
const { FSReqCallback } = internalBinding('fs');
12+
13+
// The CVE primarily affects Windows but we should test on all platforms
14+
15+
{
16+
tmpdir.refresh();
17+
}
18+
19+
{
20+
const longFileNamePart = 'a'.repeat(200);
21+
const fileName = tmpdir.resolve(`long-file-name-${longFileNamePart}-for-memory-leak-test.txt`);
22+
fs.writeFileSync(fileName, 'test content', 'utf8');
23+
const fullPath = path.resolve(fileName);
24+
25+
assert(fs.existsSync(fullPath), 'Test file should exist');
26+
27+
async function runTest() {
28+
try {
29+
await checkIfCollectableByCounting(
30+
() => {
31+
for (let i = 0; i < 10; i++) {
32+
fs.existsSync(fullPath);
33+
}
34+
return 10;
35+
},
36+
FSReqCallback,
37+
10
38+
);
39+
} catch (err) {
40+
assert.ifError(err, 'Memory leak detected: FSReqCallback objects were not collected');
41+
} finally {
42+
tmpdir.refresh();
43+
}
44+
}
45+
46+
runTest().then(common.mustCall());
47+
}

0 commit comments

Comments
 (0)