From 90ed832293135879dd78f6b30c259c3c20076ec2 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 18 Feb 2021 18:56:32 +0530 Subject: [PATCH 1/4] fs: validate mode in ReadStream and WriteStream Fixes: https://github.com/nodejs/node/issues/37430 --- lib/internal/fs/streams.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index 2e79243da8fb5e..08e8ffaa5682e1 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -18,6 +18,7 @@ const { } = require('internal/errors').codes; const { deprecate } = require('internal/util'); const { + parseFileMode, validateFunction, validateInteger, } = require('internal/validators'); @@ -168,7 +169,7 @@ function ReadStream(path, options) { // Path will be ignored when fd is specified, so it can be falsy this.path = toPathIfFileURL(path); this.flags = options.flags === undefined ? 'r' : options.flags; - this.mode = options.mode === undefined ? 0o666 : options.mode; + this.mode = parseFileMode(options.mode, 'options.mode', 0o666); importFd(this, options); @@ -334,7 +335,7 @@ function WriteStream(path, options) { // Path will be ignored when fd is specified, so it can be falsy this.path = toPathIfFileURL(path); this.flags = options.flags === undefined ? 'w' : options.flags; - this.mode = options.mode === undefined ? 0o666 : options.mode; + this.mode = parseFileMode(options.mode, 'options.mode', 0o666); importFd(this, options); From 95479ffbe151d38a4f4368e5639358e8fd6a257f Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 18 Feb 2021 19:33:51 +0530 Subject: [PATCH 2/4] add tests --- .../test-fs-read-stream-throw-type-error.js | 3 +++ .../test-fs-write-stream-throw-range-error.js | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 test/parallel/test-fs-write-stream-throw-range-error.js diff --git a/test/parallel/test-fs-read-stream-throw-type-error.js b/test/parallel/test-fs-read-stream-throw-type-error.js index 83b9387cc38da9..173f0083671350 100644 --- a/test/parallel/test-fs-read-stream-throw-type-error.js +++ b/test/parallel/test-fs-read-stream-throw-type-error.js @@ -75,3 +75,6 @@ const NOT_SAFE_INTEGER = 2 ** 53; ].forEach((opts) => createReadStreamErr(example, opts, rangeError) ); + +// Case 8: Should throw RangeError if mode is out of range +createReadStreamErr(example, { mode: 2176057344 }, rangeError); diff --git a/test/parallel/test-fs-write-stream-throw-range-error.js b/test/parallel/test-fs-write-stream-throw-range-error.js new file mode 100644 index 00000000000000..3beb7196cca113 --- /dev/null +++ b/test/parallel/test-fs-write-stream-throw-range-error.js @@ -0,0 +1,24 @@ +'use strict'; +require('../common'); + +// This test ensures that createWriteStream throws a RangeError when +// an out of range mode is passed in the options, as reported here: +// https://github.com/nodejs/node/issues/37430 + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const example = path.join(tmpdir.path, 'dummy'); + +assert.throws( + () => { + fs.createWriteStream(example, { mode: 2176057344 }); + }, + { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError' + }); From 49132cc9b3dd7a268387b36a0b785d606de33b5d Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 18 Feb 2021 21:37:01 +0530 Subject: [PATCH 3/4] mask mode with 0xFFFFFFFF --- lib/internal/validators.js | 2 +- .../test-fs-read-stream-throw-type-error.js | 4 ++-- ...-fs-write-stream-huge-unsigned-int-mode.js | 16 +++++++++++++ .../test-fs-write-stream-throw-range-error.js | 24 ------------------- 4 files changed, 19 insertions(+), 27 deletions(-) create mode 100644 test/parallel/test-fs-write-stream-huge-unsigned-int-mode.js delete mode 100644 test/parallel/test-fs-write-stream-throw-range-error.js diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 75efbf5c8a5186..a4c92eee97379b 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -61,7 +61,7 @@ function parseFileMode(value, name, def) { } if (isUint32(value)) { - return value; + return 0xFFFFFFFF & value; } if (typeof value === 'number') { diff --git a/test/parallel/test-fs-read-stream-throw-type-error.js b/test/parallel/test-fs-read-stream-throw-type-error.js index 173f0083671350..bb5903cc6222ec 100644 --- a/test/parallel/test-fs-read-stream-throw-type-error.js +++ b/test/parallel/test-fs-read-stream-throw-type-error.js @@ -76,5 +76,5 @@ const NOT_SAFE_INTEGER = 2 ** 53; createReadStreamErr(example, opts, rangeError) ); -// Case 8: Should throw RangeError if mode is out of range -createReadStreamErr(example, { mode: 2176057344 }, rangeError); +// Case 8: Should not throw any error even if mode is a huge unsigned int32 +fs.createReadStream(example, { mode: 2176057344 }); diff --git a/test/parallel/test-fs-write-stream-huge-unsigned-int-mode.js b/test/parallel/test-fs-write-stream-huge-unsigned-int-mode.js new file mode 100644 index 00000000000000..143df0157cb8b3 --- /dev/null +++ b/test/parallel/test-fs-write-stream-huge-unsigned-int-mode.js @@ -0,0 +1,16 @@ +'use strict'; +require('../common'); + +// This test ensures that createWriteStream does not crash when the +// passed mode is a huge unsigned int32, as reported here: +// https://github.com/nodejs/node/issues/37430 + +const fs = require('fs'); +const path = require('path'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const example = path.join(tmpdir.path, 'dummy'); + +fs.createWriteStream(example, { mode: 2176057344 }); diff --git a/test/parallel/test-fs-write-stream-throw-range-error.js b/test/parallel/test-fs-write-stream-throw-range-error.js deleted file mode 100644 index 3beb7196cca113..00000000000000 --- a/test/parallel/test-fs-write-stream-throw-range-error.js +++ /dev/null @@ -1,24 +0,0 @@ -'use strict'; -require('../common'); - -// This test ensures that createWriteStream throws a RangeError when -// an out of range mode is passed in the options, as reported here: -// https://github.com/nodejs/node/issues/37430 - -const assert = require('assert'); -const fs = require('fs'); -const path = require('path'); - -const tmpdir = require('../common/tmpdir'); -tmpdir.refresh(); - -const example = path.join(tmpdir.path, 'dummy'); - -assert.throws( - () => { - fs.createWriteStream(example, { mode: 2176057344 }); - }, - { - code: 'ERR_OUT_OF_RANGE', - name: 'RangeError' - }); From e2760b574157cea14713e11ecdfbab908136704c Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Fri, 19 Feb 2021 19:09:51 +0530 Subject: [PATCH 4/4] Revert "fs: validate mode in ReadStream and WriteStream" This reverts commit 90ed832293135879dd78f6b30c259c3c20076ec2. --- lib/internal/fs/streams.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index 08e8ffaa5682e1..2e79243da8fb5e 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -18,7 +18,6 @@ const { } = require('internal/errors').codes; const { deprecate } = require('internal/util'); const { - parseFileMode, validateFunction, validateInteger, } = require('internal/validators'); @@ -169,7 +168,7 @@ function ReadStream(path, options) { // Path will be ignored when fd is specified, so it can be falsy this.path = toPathIfFileURL(path); this.flags = options.flags === undefined ? 'r' : options.flags; - this.mode = parseFileMode(options.mode, 'options.mode', 0o666); + this.mode = options.mode === undefined ? 0o666 : options.mode; importFd(this, options); @@ -335,7 +334,7 @@ function WriteStream(path, options) { // Path will be ignored when fd is specified, so it can be falsy this.path = toPathIfFileURL(path); this.flags = options.flags === undefined ? 'w' : options.flags; - this.mode = parseFileMode(options.mode, 'options.mode', 0o666); + this.mode = options.mode === undefined ? 0o666 : options.mode; importFd(this, options);