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 <marcoippolito54@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
1 parent 02031a9 commit e07adb3

2 files changed

Lines changed: 48 additions & 0 deletions

File tree

‎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+
constcommon=require('../common');
5+
const{ checkIfCollectableByCounting }=require('../common/gc');
6+
constassert=require('assert');
7+
constfs=require('node:fs');
8+
constpath=require('node:path');
9+
consttmpdir=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+
constlongFileNamePart='a'.repeat(200);
21+
constfileName=tmpdir.resolve(`long-file-name-${longFileNamePart}-for-memory-leak-test.txt`);
22+
fs.writeFileSync(fileName,'test content','utf8');
23+
constfullPath=path.resolve(fileName);
24+
25+
assert(fs.existsSync(fullPath),'Test file should exist');
26+
27+
asyncfunctionrunTest(){
28+
try{
29+
awaitcheckIfCollectableByCounting(
30+
()=>{
31+
for(leti=0;i<10;i++){
32+
fs.existsSync(fullPath);
33+
}
34+
return10;
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)