From 59eef81dc78c86246257089feef4af91e0229387 Mon Sep 17 00:00:00 2001 From: Nicolas Ramz Date: Mon, 20 Apr 2020 14:59:30 +0200 Subject: [PATCH 1/3] fix: readdir should check for access rights, fixes #294 --- lib/binding.js | 2 ++ test/lib/fs.readdir.spec.js | 45 ++++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/lib/binding.js b/lib/binding.js index 34535d54..4a29ae2b 100644 --- a/lib/binding.js +++ b/lib/binding.js @@ -963,6 +963,8 @@ Binding.prototype.readdir = function( throw new FSError('ENOTDIR', dirpath); } + this.access(dirpath, parseInt('0002', 8)); + let list = dir.list(); if (encoding === 'buffer') { list = list.map(function(item) { diff --git a/test/lib/fs.readdir.spec.js b/test/lib/fs.readdir.spec.js index 6ec68f8b..26d71d7a 100644 --- a/test/lib/fs.readdir.spec.js +++ b/test/lib/fs.readdir.spec.js @@ -21,7 +21,15 @@ describe('fs.readdir(path, callback)', function() { empty: {} } } - } + }, + denied: mock.directory({ + mode: 0o000, + items: [ + { + 'one.txt': 'content' + } + ] + }) }); }); afterEach(mock.restore); @@ -92,6 +100,29 @@ describe('fs.readdir(path, callback)', function() { ); }); + it('calls with an error for restricted path', function(done) { + fs.readdir('denied', function(err, items) { + assert.instanceOf(err, Error); + assert.isUndefined(items); + done(); + }); + }); + + withPromise.it('promise calls with an error for restricted path', function( + done + ) { + fs.promises.readdir('denied').then( + function() { + assert.fail('should not succeed.'); + done(); + }, + function(err) { + assert.instanceOf(err, Error); + done(); + } + ); + }); + inVersion('>=10.10').it('should support "withFileTypes" option', function( done ) { @@ -216,4 +247,16 @@ describe('fs.readdirSync(path)', function() { fs.readdirSync('bogus'); }); }); + + it('throws when access refused', function() { + assert.throws(function() { + fs.readdirSync('denied'); + }); + }); + + it('throws when access refused', function() { + assert.throws(function() { + fs.readdirSync('denied'); + }); + }); }); From 56801daffe1bd2bb81ef4043b187df109fe1befa Mon Sep 17 00:00:00 2001 From: Nicolas Ramz Date: Mon, 20 Apr 2020 18:25:39 +0200 Subject: [PATCH 2/3] fixed check --- lib/binding.js | 5 +++-- test/lib/fs.readdir.spec.js | 6 ------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/binding.js b/lib/binding.js index 4a29ae2b..0e34b9e4 100644 --- a/lib/binding.js +++ b/lib/binding.js @@ -962,8 +962,9 @@ Binding.prototype.readdir = function( if (!(dir instanceof Directory)) { throw new FSError('ENOTDIR', dirpath); } - - this.access(dirpath, parseInt('0002', 8)); + if (!dir.canRead()) { + throw new FSError('EACCES', dirpath); + } let list = dir.list(); if (encoding === 'buffer') { diff --git a/test/lib/fs.readdir.spec.js b/test/lib/fs.readdir.spec.js index 26d71d7a..fde648aa 100644 --- a/test/lib/fs.readdir.spec.js +++ b/test/lib/fs.readdir.spec.js @@ -253,10 +253,4 @@ describe('fs.readdirSync(path)', function() { fs.readdirSync('denied'); }); }); - - it('throws when access refused', function() { - assert.throws(function() { - fs.readdirSync('denied'); - }); - }); }); From f0ce80352690ede2ee319fedf178bf8260d203a5 Mon Sep 17 00:00:00 2001 From: Nicolas Ramz Date: Tue, 21 Apr 2020 09:33:55 +0200 Subject: [PATCH 3/3] made readdir access test more resiliant --- test/lib/fs.readdir.spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/lib/fs.readdir.spec.js b/test/lib/fs.readdir.spec.js index fde648aa..5b531c30 100644 --- a/test/lib/fs.readdir.spec.js +++ b/test/lib/fs.readdir.spec.js @@ -103,6 +103,7 @@ describe('fs.readdir(path, callback)', function() { it('calls with an error for restricted path', function(done) { fs.readdir('denied', function(err, items) { assert.instanceOf(err, Error); + assert.equal(err.code, 'EACCES'); assert.isUndefined(items); done(); }); @@ -118,6 +119,7 @@ describe('fs.readdir(path, callback)', function() { }, function(err) { assert.instanceOf(err, Error); + assert.equal(err.code, 'EACCES'); done(); } );