Skip to content

Commit 9c06103

Browse files
jasnelltargos
authored andcommitted
src: fix validation of negative offset to avoid abort
Fixes: #24640 Signed-off-by: James M Snell <[email protected]> PR-URL: #38421 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Nitzan Uziely <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
1 parent 6350d35 commit 9c06103

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
@@ -630,7 +630,7 @@ function read(fd, buffer, offset, length, position, callback) {
630630
if (offset == null) {
631631
offset = 0;
632632
} else {
633-
validateInteger(offset, 'offset');
633+
validateInteger(offset, 'offset', 0);
634634
}
635635

636636
length |= 0;
@@ -694,7 +694,7 @@ function readSync(fd, buffer, offset, length, position) {
694694
if (offset == null) {
695695
offset = 0;
696696
} else {
697-
validateInteger(offset, 'offset');
697+
validateInteger(offset, 'offset', 0);
698698
}
699699

700700
length |= 0;
@@ -806,7 +806,7 @@ function write(fd, buffer, offset, length, position, callback) {
806806
if (offset == null || typeof offset === 'function') {
807807
offset = 0;
808808
} else {
809-
validateInteger(offset, 'offset');
809+
validateInteger(offset, 'offset', 0);
810810
}
811811
if (typeof length !== 'number')
812812
length = buffer.byteLength - offset;
@@ -863,7 +863,7 @@ function writeSync(fd, buffer, offset, length, position) {
863863
if (offset == null) {
864864
offset = 0;
865865
} else {
866-
validateInteger(offset, 'offset');
866+
validateInteger(offset, 'offset', 0);
867867
}
868868
if (typeof length !== 'number')
869869
length = buffer.byteLength - offset;

lib/internal/fs/promises.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ async function read(handle, bufferOrOptions, offset, length, position) {
417417
if (offset == null) {
418418
offset = 0;
419419
} else {
420-
validateInteger(offset, 'offset');
420+
validateInteger(offset, 'offset', 0);
421421
}
422422

423423
length |= 0;
@@ -460,7 +460,7 @@ async function write(handle, buffer, offset, length, position) {
460460
if (offset == null) {
461461
offset = 0;
462462
} else {
463-
validateInteger(offset, 'offset');
463+
validateInteger(offset, 'offset', 0);
464464
}
465465
if (typeof length !== 'number')
466466
length = buffer.byteLength - offset;

lib/internal/fs/utils.js

+4
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,10 @@ const validateOffsetLengthWrite = hideStackFrames(
655655
if (length > byteLength - offset) {
656656
throw new ERR_OUT_OF_RANGE('length', `<= ${byteLength - offset}`, length);
657657
}
658+
659+
if (length < 0) {
660+
throw new ERR_OUT_OF_RANGE('length', '>= 0', length);
661+
}
658662
}
659663
);
660664

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(() => {
@@ -157,8 +155,6 @@ assert.throws(() => {
157155
}, {
158156
code: 'ERR_OUT_OF_RANGE',
159157
name: 'RangeError',
160-
message: 'The value of "offset" is out of range. ' +
161-
'It must be >= 0. Received -1'
162158
});
163159

164160
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)