node icon indicating copy to clipboard operation
node copied to clipboard

Buffer.toString() cannot handle large indices

Open imyp92 opened this issue 4 months ago • 5 comments

Version

14.17.3

Platform

23.4.0 Darwin Kernel Version 23.4.0

Subsystem

No response

What steps will reproduce the bug?

let buffer = Buffer.alloc(2279415336);

let res = buffer.toString('utf8', 2147483648, 2147483700); // 2^32 - 1 < start

// buffer.js:605
//     slice: (buf, start, end) => buf.utf8Slice(start, end),
                                    ^

// RangeError: Index out of range

How often does it reproduce? Is there a required condition?

everytime

What is the expected behavior? Why is that the expected behavior?

Buffer.toString() should be able to handle buffers smaller than kMaxLength.

What do you see instead?

run without an error

Additional information

The bitwise or assignment (|=) operation of Buffer.toString() seems to be the cause of the error. If start or end parameter greater than INT_MAX is passed, the value changes to a negative number, resulting in an index out of range error.

imyp92 avatar Apr 01 '24 02:04 imyp92

The issue you're encountering is due to JavaScript's handling of integers beyond Number.MAX_SAFE_INTEGER (which is 2^53 - 1), resulting in the values being interpreted as negative when passed to Buffer.toString().

make sure that the start and end parameters passed to Buffer.toString() are within the range of safe integers. You can achieve this by performing bounds checking before calling Buffer.toString().

Malikrehman00107 avatar Apr 01 '24 06:04 Malikrehman00107

The issue you're encountering is due to JavaScript's handling of integers beyond Number.MAX_SAFE_INTEGER (which is 2^53 - 1), resulting in the values being interpreted as negative when passed to Buffer.toString().

make sure that the start and end parameters passed to Buffer.toString() are within the range of safe integers. You can achieve this by performing bounds checking before calling Buffer.toString().

Sorry for being made an unclear comment on example code. I wrote INT_MAX < start but I think it would be correct to express it as 2^32-1. This problem occurs when using a value greater than 2^31-1 as offsets(start or end). And this is smaller than the kMaxLength constant, which is the maximum size of buffer allowed by Buffer.js (in 64-bit architecture, 2^32). so I think this case should be handled by Buffer.toString().

imyp92 avatar Apr 01 '24 07:04 imyp92

I see !

you may need to perform bounds checking before calling Buffer.toString() to ensure that the offsets are within the range of safe integers. This will help prevent the index out of range errors until a fix or enhancement is made to Buffer.toString().

Rest i think Node Js internal need to have a look on this.

Malikrehman00107 avatar Apr 01 '24 07:04 Malikrehman00107

This should fix it in a non-breaking way:

diff --git a/lib/buffer.js b/lib/buffer.js
index a8d07342e15..904f1fff6b3 100644
--- a/lib/buffer.js
+++ b/lib/buffer.js
@@ -820,12 +820,12 @@ Buffer.prototype.toString = function toString(encoding, start, end) {
   else if (start >= len)
     return '';
   else
-    start |= 0;
+    start = MathTrunc(start) || 0;

   if (end === undefined || end > len)
     end = len;
   else
-    end |= 0;
+    end = MathTrunc(end) || 0;

   if (end <= start)
     return '';

targos avatar Apr 01 '24 07:04 targos

Thank you @targos !

Malikrehman00107 avatar Apr 01 '24 07:04 Malikrehman00107