Skip to content

Commit 378e0e7

Browse files
jasnelltargos
authored andcommitted
src: fix validation of negative offset to avoid abort
Fixes: #24640 Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #38421 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Nitzan Uziely <linkgoron@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
1 parent 3484a23 commit 378e0e7

File tree

5 files changed

+65
-10
lines changed

5 files changed

+65
-10
lines changed

lib/fs.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ function read(fd, buffer, offset, length, position, callback) {
535535
if (offset == null) {
536536
offset = 0;
537537
} else {
538-
validateInteger(offset, 'offset');
538+
validateInteger(offset, 'offset', 0);
539539
}
540540

541541
length |= 0;
@@ -589,7 +589,7 @@ function readSync(fd, buffer, offset, length, position) {
589589
if (offset == null) {
590590
offset = 0;
591591
} else {
592-
validateInteger(offset, 'offset');
592+
validateInteger(offset, 'offset', 0);
593593
}
594594

595595
length |= 0;
@@ -667,7 +667,7 @@ function write(fd, buffer, offset, length, position, callback) {
667667
if (offset == null || typeof offset === 'function') {
668668
offset = 0;
669669
} else {
670-
validateInteger(offset, 'offset');
670+
validateInteger(offset, 'offset', 0);
671671
}
672672
if (typeof length !== 'number')
673673
length = buffer.length - offset;
@@ -715,7 +715,7 @@ function writeSync(fd, buffer, offset, length, position) {
715715
if (offset == null) {
716716
offset = 0;
717717
} else {
718-
validateInteger(offset, 'offset');
718+
validateInteger(offset, 'offset', 0);
719719
}
720720
if (typeof length !== 'number')
721721
length = buffer.byteLength - offset;

lib/internal/fs/promises.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ async function read(handle, bufferOrOptions, offset, length, position) {
366366
if (offset == null) {
367367
offset = 0;
368368
} else {
369-
validateInteger(offset, 'offset');
369+
validateInteger(offset, 'offset', 0);
370370
}
371371

372372
length |= 0;
@@ -409,7 +409,7 @@ async function write(handle, buffer, offset, length, position) {
409409
if (offset == null) {
410410
offset = 0;
411411
} else {
412-
validateInteger(offset, 'offset');
412+
validateInteger(offset, 'offset', 0);
413413
}
414414
if (typeof length !== 'number')
415415
length = buffer.length - offset;

lib/internal/fs/utils.js

+4
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,10 @@ const validateOffsetLengthWrite = hideStackFrames(
620620
if (length > byteLength - offset) {
621621
throw new ERR_OUT_OF_RANGE('length', `<= ${byteLength - offset}`, length);
622622
}
623+
624+
if (length < 0) {
625+
throw new ERR_OUT_OF_RANGE('length', '>= 0', length);
626+
}
623627
}
624628
);
625629

test/parallel/test-fs-read-type.js

-4
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ assert.throws(() => {
4444
}, {
4545
code: 'ERR_OUT_OF_RANGE',
4646
name: 'RangeError',
47-
message: 'The value of "offset" is out of range. It must be >= 0. ' +
48-
'Received -1'
4947
});
5048

5149
assert.throws(() => {
@@ -109,8 +107,6 @@ assert.throws(() => {
109107
}, {
110108
code: 'ERR_OUT_OF_RANGE',
111109
name: 'RangeError',
112-
message: 'The value of "offset" is out of range. ' +
113-
'It must be >= 0. Received -1'
114110
});
115111

116112
assert.throws(() => {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
'use strict';
2+
3+
// Tests that passing a negative offset does not crash the process
4+
5+
const common = require('../common');
6+
7+
const {
8+
join,
9+
} = require('path');
10+
11+
const {
12+
closeSync,
13+
open,
14+
write,
15+
writeSync,
16+
} = require('fs');
17+
18+
const assert = require('assert');
19+
20+
const tmpdir = require('../common/tmpdir');
21+
tmpdir.refresh();
22+
23+
const filename = join(tmpdir.path, 'test.txt');
24+
25+
open(filename, 'w+', common.mustSucceed((fd) => {
26+
assert.throws(() => {
27+
write(fd, Buffer.alloc(0), -1, common.mustNotCall());
28+
}, {
29+
code: 'ERR_OUT_OF_RANGE',
30+
});
31+
assert.throws(() => {
32+
writeSync(fd, Buffer.alloc(0), -1);
33+
}, {
34+
code: 'ERR_OUT_OF_RANGE',
35+
});
36+
closeSync(fd);
37+
}));
38+
39+
const filename2 = join(tmpdir.path, 'test2.txt');
40+
41+
// Make sure negative length's don't cause aborts either
42+
43+
open(filename2, 'w+', common.mustSucceed((fd) => {
44+
assert.throws(() => {
45+
write(fd, Buffer.alloc(0), 0, -1, common.mustNotCall());
46+
}, {
47+
code: 'ERR_OUT_OF_RANGE',
48+
});
49+
assert.throws(() => {
50+
writeSync(fd, Buffer.alloc(0), 0, -1);
51+
}, {
52+
code: 'ERR_OUT_OF_RANGE',
53+
});
54+
closeSync(fd);
55+
}));

0 commit comments

Comments
 (0)