Fast fail array decoding (#132)

* fast fail when array length exceeds bytes left to decode

Author: Ryang-21

CHANGELOG.md

@@ -11,6 +11,8 @@ project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 * Removed [the custom `Buffer.subarray` polyfill](https://github.com/stellar/js-xdr/pull/118) introduced in v3.1.1 to address the issue of it not being usable in the React Native Hermes engine. We recommend using [`@exodus/patch-broken-hermes-typed-arrays`](https://github.com/ExodusMovement/patch-broken-hermes-typed-arrays) as an alternative. If needed, please review and consider manually adding it to your project ([#128](https://github.com/stellar/js-xdr/pull/128)).
 
+* Decoding Array and VarArray now fast fails when the array length exceeds remaining bytes to decode ([#132](https://github.com/stellar/js-xdr/pull/132))
+
 ## [v3.1.2](https://github.com/stellar/js-xdr/compare/v3.1.1...v3.1.2)
 
 ### Fixed

src/array.js

@@ -1,5 +1,5 @@
 import { NestedXdrType } from './xdr-type';
-import { XdrWriterError } from './errors';
+import { XdrWriterError, XdrReaderError } from './errors';
 
 export class Array extends NestedXdrType {
   constructor(childType, length, maxDepth = NestedXdrType.DEFAULT_MAX_DEPTH) {
@@ -12,6 +12,15 @@ export class Array extends NestedXdrType {
    * @inheritDoc
    */
   read(reader, remainingDepth = this._maxDepth) {
+    // Upper-bound fast-fail: remaining bytes is a loose capacity check since
+    // each XDR element typically consumes more than 1 byte (e.g., 4+ bytes).
+    if (this._length > reader.remainingBytes()) {
+      throw new XdrReaderError(
+        `Array length ${
+          this._length
+        } exceeds remaining ${reader.remainingBytes()} bytes`
+      );
+    }
     NestedXdrType.checkDepth(remainingDepth);
     // allocate array of specified length
     const result = new global.Array(this._length);

src/serialization/xdr-reader.js

@@ -88,6 +88,14 @@ export class XdrReader {
     this._index = 0;
   }
 
+  /**
+   * Remaining unread bytes in the source buffer
+   * @return {Number}
+   */
+  remainingBytes() {
+    return this._length - this._index;
+  }
+
   /**
    * Read byte array from the buffer
    * @param {Number} size - Bytes to read

src/var-array.js

@@ -24,6 +24,14 @@ export class VarArray extends NestedXdrType {
         `saw ${length} length VarArray, max allowed is ${this._maxLength}`
       );
 
+    // Upper-bound fast-fail: remaining bytes is a loose capacity check since
+    // each XDR element typically consumes more than 1 byte (e.g., 4+ bytes)
+    if (length > reader.remainingBytes()) {
+      throw new XdrReaderError(
+        `VarArray length ${length} exceeds remaining ${reader.remainingBytes()} bytes`
+      );
+    }
+
     const result = new Array(length);
     for (let i = 0; i < length; i++) {
       result[i] = this._childType.read(reader, remainingDepth - 1);

test/unit/array_test.js

@@ -6,6 +6,28 @@ let one = new XDR.Array(XDR.Int, 1);
 let many = new XDR.Array(XDR.Int, 2);
 
 describe('Array#read', function () {
+  function createFixedSizeChild() {
+    let calls = 0;
+
+    return {
+      FixedSizeChild: class FixedSizeChild {
+        static read(io) {
+          calls += 1;
+          return io.readInt32BE();
+        }
+
+        static write() {}
+
+        static isValid() {
+          return true;
+        }
+      },
+      getCalls() {
+        return calls;
+      }
+    };
+  }
+
   it('decodes correctly', function () {
     expect(read(zero, [])).to.eql([]);
     expect(read(zero, [0x00, 0x00, 0x00, 0x00])).to.eql([]);
@@ -23,8 +45,54 @@ describe('Array#read', function () {
 
   it("throws XdrReaderError when the byte stream isn't large enough", function () {
     expect(() => read(many, [0x00, 0x00, 0x00, 0x00])).to.throw(
-      /read outside the boundary/i
+      /(read outside the boundary|insufficient bytes)/i
+    );
+  });
+
+  it('fast-fails before decoding child elements when remaining bytes are insufficient', function () {
+    const { FixedSizeChild, getCalls } = createFixedSizeChild();
+
+    const fixed = new XDR.Array(FixedSizeChild, 5);
+    const reader = new XdrReader([0x00, 0x00, 0x00, 0x01]);
+    expect(() => fixed.read(reader)).to.throw(
+      new RegExp(`exceeds remaining ${reader.remainingBytes()} bytes`, 'i')
+    );
+    expect(getCalls()).to.eql(0);
+  });
+
+  it('decodes on exact-fit byte length and reads each child exactly once', function () {
+    const { FixedSizeChild, getCalls } = createFixedSizeChild();
+
+    const fixed = new XDR.Array(FixedSizeChild, 2);
+    const reader = new XdrReader([
+      0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02
+    ]);
+
+    expect(fixed.read(reader)).to.eql([1, 2]);
+    expect(getCalls()).to.eql(2);
+  });
+
+  it('zero-length array does not decode child elements', function () {
+    const { FixedSizeChild, getCalls } = createFixedSizeChild();
+
+    const fixed = new XDR.Array(FixedSizeChild, 0);
+    const reader = new XdrReader([0x01, 0x02, 0x03, 0x04]);
+
+    expect(fixed.read(reader)).to.eql([]);
+    expect(getCalls()).to.eql(0);
+  });
+
+  it('keeps reader position unchanged on Array fast-fail', function () {
+    const { FixedSizeChild } = createFixedSizeChild();
+
+    const fixed = new XDR.Array(FixedSizeChild, 3);
+    const reader = new XdrReader([0x00, 0x00]);
+    const before = reader.remainingBytes();
+
+    expect(() => fixed.read(reader)).to.throw(
+      new RegExp(`exceeds remaining ${reader.remainingBytes()} bytes`, 'i')
     );
+    expect(reader.remainingBytes()).to.eql(before);
   });
 
   function read(arr, bytes) {

test/unit/var-array_test.js

@@ -4,6 +4,28 @@ import { XdrWriter } from '../../src/serialization/xdr-writer';
 const subject = new XDR.VarArray(XDR.Int, 2);
 
 describe('VarArray#read', function () {
+  function createFixedSizeChild() {
+    let calls = 0;
+
+    return {
+      FixedSizeChild: class FixedSizeChild {
+        static read(io) {
+          calls += 1;
+          return io.readInt32BE();
+        }
+
+        static write() {}
+
+        static isValid() {
+          return true;
+        }
+      },
+      getCalls() {
+        return calls;
+      }
+    };
+  }
+
   it('decodes correctly', function () {
     expect(read([0x00, 0x00, 0x00, 0x00])).to.eql([]);
 
@@ -26,6 +48,55 @@ describe('VarArray#read', function () {
     expect(() => read([0x00, 0x00, 0x00, 0x03])).to.throw(/read error/i);
   });
 
+  it('fast-fails when declared length cannot fit remaining bytes for fixed-size child', function () {
+    const { FixedSizeChild, getCalls } = createFixedSizeChild();
+
+    const arr = new XDR.VarArray(FixedSizeChild, 10);
+    const io = new XdrReader([0x00, 0x00, 0x00, 0x02]);
+    const remainingAfterLengthPrefix = io.remainingBytes() - 4;
+
+    expect(() => arr.read(io)).to.throw(
+      new RegExp(`exceeds remaining ${remainingAfterLengthPrefix} bytes`, 'i')
+    );
+    expect(getCalls()).to.eql(0);
+  });
+
+  it('decodes on exact-fit payload and reads each child once', function () {
+    const { FixedSizeChild, getCalls } = createFixedSizeChild();
+
+    const arr = new XDR.VarArray(FixedSizeChild, 10);
+    const io = new XdrReader([
+      0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02
+    ]);
+
+    expect(arr.read(io)).to.eql([1, 2]);
+    expect(getCalls()).to.eql(2);
+  });
+
+  it('checks maxLength before insufficient-bytes fast-fail', function () {
+    const { FixedSizeChild, getCalls } = createFixedSizeChild();
+
+    const arr = new XDR.VarArray(FixedSizeChild, 1);
+    const io = new XdrReader([0x00, 0x00, 0x00, 0x02]);
+
+    expect(() => arr.read(io)).to.throw(/max allowed/i);
+    expect(getCalls()).to.eql(0);
+  });
+
+  it('consumes only the 4-byte length prefix on VarArray insufficient-bytes fast-fail', function () {
+    const { FixedSizeChild } = createFixedSizeChild();
+
+    const arr = new XDR.VarArray(FixedSizeChild, 10);
+    const io = new XdrReader([0x00, 0x00, 0x00, 0x02]);
+    const before = io.remainingBytes();
+    const remainingAfterLengthPrefix = io.remainingBytes() - 4;
+
+    expect(() => arr.read(io)).to.throw(
+      new RegExp(`exceeds remaining ${remainingAfterLengthPrefix} bytes`, 'i')
+    );
+    expect(before - io.remainingBytes()).to.eql(4);
+  });
+
   function read(bytes) {
     let io = new XdrReader(bytes);
     return subject.read(io);