From 63a0887f35ae837e4e73cc05640b029dcb23d6d6 Mon Sep 17 00:00:00 2001 From: Nitzan Uziely Date: Fri, 22 Oct 2021 20:00:45 +0300 Subject: [PATCH 1/3] dgram: fix send with out of bounds offset + length fix Socket.prototype.send sending garbage when the message is a string, or Buffer and offset+length is out of bounds. Fixes: https://github.com/nodejs/node/issues/40491 --- lib/dgram.js | 13 ++++ .../parallel/test-dgram-send-bad-arguments.js | 65 +++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/lib/dgram.js b/lib/dgram.js index 931d9a49544dcc..202d33636893af 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -42,6 +42,7 @@ const { guessHandleType } = internalBinding('util'); const { ERR_INVALID_ARG_TYPE, ERR_MISSING_ARGS, + ERR_OUT_OF_RANGE, ERR_SOCKET_ALREADY_BOUND, ERR_SOCKET_BAD_BUFFER_SIZE, ERR_SOCKET_BUFFER_SIZE, @@ -488,6 +489,18 @@ function sliceBuffer(buffer, offset, length) { offset = offset >>> 0; length = length >>> 0; + // TypedArray and DataView bounds are checked in Buffer.from + if (Buffer.isBuffer(buffer)) { + if (offset > buffer.byteLength) { + throw new ERR_OUT_OF_RANGE('offset', `<= ${buffer.byteLength}`, offset); + } + + if (offset + length > buffer.byteLength) { + throw new ERR_OUT_OF_RANGE('length', + `<= ${buffer.byteLength - offset}`, length); + } + } + return Buffer.from(buffer.buffer, buffer.byteOffset + offset, length); } diff --git a/test/parallel/test-dgram-send-bad-arguments.js b/test/parallel/test-dgram-send-bad-arguments.js index 3e42f31b1af4b6..fa250cfc4e0da0 100644 --- a/test/parallel/test-dgram-send-bad-arguments.js +++ b/test/parallel/test-dgram-send-bad-arguments.js @@ -77,6 +77,71 @@ function checkArgs(connected) { message: 'Already connected' } ); + + for (const input of ['hello', Buffer.from('hello'), + Buffer.from('hello world').subarray(0, 5), + Buffer.from('hello world').subarray(6)]) { + assert.throws( + () => { sock.send(input, 6, 0); }, + { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "offset" is out of range. ' + + 'It must be <= 5. Received 6' + } + ); + + assert.throws( + () => { sock.send(input, 0, 6); }, + { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "length" is out of range. ' + + 'It must be <= 5. Received 6' + } + ); + + assert.throws( + () => { sock.send(input, 3, 4); }, + { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "length" is out of range. ' + + 'It must be <= 2. Received 4' + } + ); + } + + for (const input of [new Uint8Array([1, 2, 3, 4, 5]), + new DataView(new ArrayBuffer(5), 0), + new DataView(new ArrayBuffer(6), 2)]) { + assert.throws( + () => { sock.send(input, 6, 0); }, + { + code: 'ERR_BUFFER_OUT_OF_BOUNDS', + name: 'RangeError', + message: '"offset" is outside of buffer bounds', + } + ); + + assert.throws( + () => { sock.send(input, 0, 6); }, + { + code: 'ERR_BUFFER_OUT_OF_BOUNDS', + name: 'RangeError', + message: '"length" is outside of buffer bounds', + } + ); + + assert.throws( + () => { sock.send(input, 3, 4); }, + { + code: 'ERR_BUFFER_OUT_OF_BOUNDS', + name: 'RangeError', + message: '"length" is outside of buffer bounds', + } + ); + } } else { assert.throws(() => { sock.send(buf, 1, 1, -1, host); }, RangeError); assert.throws(() => { sock.send(buf, 1, 1, 0, host); }, RangeError); From b392a94373e7048e5818526ffc3cb65a3c001553 Mon Sep 17 00:00:00 2001 From: Nitzan Uziely Date: Sat, 23 Oct 2021 01:01:54 +0300 Subject: [PATCH 2/3] fixup! dgram: fix send with out of bounds offset + length --- lib/dgram.js | 17 +++---- .../parallel/test-dgram-send-bad-arguments.js | 45 +++++-------------- 2 files changed, 16 insertions(+), 46 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index 202d33636893af..eb26173c6aca4f 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -40,9 +40,9 @@ const { } = require('internal/dgram'); const { guessHandleType } = internalBinding('util'); const { + ERR_BUFFER_OUT_OF_BOUNDS, ERR_INVALID_ARG_TYPE, ERR_MISSING_ARGS, - ERR_OUT_OF_RANGE, ERR_SOCKET_ALREADY_BOUND, ERR_SOCKET_BAD_BUFFER_SIZE, ERR_SOCKET_BUFFER_SIZE, @@ -488,17 +488,12 @@ function sliceBuffer(buffer, offset, length) { offset = offset >>> 0; length = length >>> 0; + if (offset > buffer.byteLength) { + throw new ERR_BUFFER_OUT_OF_BOUNDS('offset'); + } - // TypedArray and DataView bounds are checked in Buffer.from - if (Buffer.isBuffer(buffer)) { - if (offset > buffer.byteLength) { - throw new ERR_OUT_OF_RANGE('offset', `<= ${buffer.byteLength}`, offset); - } - - if (offset + length > buffer.byteLength) { - throw new ERR_OUT_OF_RANGE('length', - `<= ${buffer.byteLength - offset}`, length); - } + if (offset + length > buffer.byteLength) { + throw new ERR_BUFFER_OUT_OF_BOUNDS('length'); } return Buffer.from(buffer.buffer, buffer.byteOffset + offset, length); diff --git a/test/parallel/test-dgram-send-bad-arguments.js b/test/parallel/test-dgram-send-bad-arguments.js index fa250cfc4e0da0..4a62166eb33de9 100644 --- a/test/parallel/test-dgram-send-bad-arguments.js +++ b/test/parallel/test-dgram-send-bad-arguments.js @@ -78,43 +78,18 @@ function checkArgs(connected) { } ); - for (const input of ['hello', Buffer.from('hello'), + for (const input of ['hello', + Buffer.from('hello'), Buffer.from('hello world').subarray(0, 5), - Buffer.from('hello world').subarray(6)]) { - assert.throws( - () => { sock.send(input, 6, 0); }, - { - code: 'ERR_OUT_OF_RANGE', - name: 'RangeError', - message: 'The value of "offset" is out of range. ' + - 'It must be <= 5. Received 6' - } - ); - - assert.throws( - () => { sock.send(input, 0, 6); }, - { - code: 'ERR_OUT_OF_RANGE', - name: 'RangeError', - message: 'The value of "length" is out of range. ' + - 'It must be <= 5. Received 6' - } - ); - - assert.throws( - () => { sock.send(input, 3, 4); }, - { - code: 'ERR_OUT_OF_RANGE', - name: 'RangeError', - message: 'The value of "length" is out of range. ' + - 'It must be <= 2. Received 4' - } - ); - } - - for (const input of [new Uint8Array([1, 2, 3, 4, 5]), + Buffer.from('hello world').subarray(4, 9), + Buffer.from('hello world').subarray(6), + new Uint8Array([1, 2, 3, 4, 5]), + new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]).slice(0, 5), + new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]).slice(2, 7), + new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]).slice(3), new DataView(new ArrayBuffer(5), 0), - new DataView(new ArrayBuffer(6), 2)]) { + new DataView(new ArrayBuffer(6), 1), + new DataView(new ArrayBuffer(7), 1, 5)]) { assert.throws( () => { sock.send(input, 6, 0); }, { From c8295526df2ae0e8f900afc68ea0f1116d1fa146 Mon Sep 17 00:00:00 2001 From: Nitzan Uziely Date: Sun, 24 Oct 2021 19:30:37 +0300 Subject: [PATCH 3/3] fixup! fixup! dgram: fix send with out of bounds offset + length --- test/parallel/test-dgram-send-bad-arguments.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-dgram-send-bad-arguments.js b/test/parallel/test-dgram-send-bad-arguments.js index 4a62166eb33de9..b30951d441cbd2 100644 --- a/test/parallel/test-dgram-send-bad-arguments.js +++ b/test/parallel/test-dgram-send-bad-arguments.js @@ -78,15 +78,16 @@ function checkArgs(connected) { } ); + const longArray = [1, 2, 3, 4, 5, 6, 7, 8]; for (const input of ['hello', Buffer.from('hello'), Buffer.from('hello world').subarray(0, 5), Buffer.from('hello world').subarray(4, 9), Buffer.from('hello world').subarray(6), new Uint8Array([1, 2, 3, 4, 5]), - new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]).slice(0, 5), - new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]).slice(2, 7), - new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]).slice(3), + new Uint8Array(longArray).subarray(0, 5), + new Uint8Array(longArray).subarray(2, 7), + new Uint8Array(longArray).subarray(3), new DataView(new ArrayBuffer(5), 0), new DataView(new ArrayBuffer(6), 1), new DataView(new ArrayBuffer(7), 1, 5)]) {