From 8ac03699aa3808e022a3a45be06ea1d68ddfc7af Mon Sep 17 00:00:00 2001
From: Nathan Nichols <natenichols@cox.net>
Date: Tue, 7 Jul 2020 13:58:24 -0500
Subject: [PATCH] Check field_code and type_code when readFieldOrdinal() is
 called (#81)

Added verification that nth and type are valid when read from BinaryParser
---
 .../src/serdes/binary-parser.ts               | 21 +++++++++++++++---
 .../test/binary-parser.test.js                | 22 ++++++++++++++-----
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/packages/ripple-binary-codec/src/serdes/binary-parser.ts b/packages/ripple-binary-codec/src/serdes/binary-parser.ts
index 1824ed85..6998b01b 100644
--- a/packages/ripple-binary-codec/src/serdes/binary-parser.ts
+++ b/packages/ripple-binary-codec/src/serdes/binary-parser.ts
@@ -107,9 +107,24 @@ class BinaryParser {
    * @return Field ordinal
    */
   readFieldOrdinal(): number {
-    const tagByte = this.readUInt8();
-    const type = (tagByte & 0xf0) >>> 4 || this.readUInt8();
-    const nth = tagByte & 0x0f || this.readUInt8();
+    let type = this.readUInt8();
+    let nth = type & 15;
+    type >>= 4;
+
+    if (type === 0) {
+      type = this.readUInt8();
+      if (type === 0 || type < 16) {
+        throw new Error("Cannot read FieldOrdinal, type_code out of range");
+      }
+    }
+
+    if (nth === 0) {
+      nth = this.readUInt8();
+      if (nth === 0 || nth < 16) {
+        throw new Error("Cannot read FieldOrdinal, field_code out of range");
+      }
+    }
+
     return (type << 16) | nth;
   }
 
diff --git a/packages/ripple-binary-codec/test/binary-parser.test.js b/packages/ripple-binary-codec/test/binary-parser.test.js
index d6f88ad5..da8b5a5a 100644
--- a/packages/ripple-binary-codec/test/binary-parser.test.js
+++ b/packages/ripple-binary-codec/test/binary-parser.test.js
@@ -50,7 +50,6 @@ function basicApiTests () {
     expect(parser.readUInt32()).toEqual(0xFFFFFFFF)
   })
 }
-basicApiTests()
 
 function transactionParsingTests () {
   const transaction = {
@@ -184,7 +183,6 @@ function transactionParsingTests () {
     expect(_.isPlainObject(jsonFromBinary)).toBe(true)
   })
 }
-transactionParsingTests()
 
 function amountParsingTests () {
   _.filter(fixtures.values_tests, { type: 'Amount' }).forEach((f, i) => {
@@ -206,7 +204,6 @@ function amountParsingTests () {
     })
   })
 }
-amountParsingTests()
 
 function fieldParsingTests () {
   fixtures.fields_tests.forEach((f, i) => {
@@ -217,8 +214,20 @@ function fieldParsingTests () {
       expect(field.type.name).toEqual(f.type_name)
     })
   })
+  test("Field throws when type code out of range", () => {
+    const parser = makeParser("0101");
+    expect(() => parser.readField()).toThrow(new Error("Cannot read FieldOrdinal, type_code out of range"));
+  })
+  test("Field throws when field code out of range", () => {
+    const parser = makeParser("1001");
+    expect(() => parser.readFieldOrdinal()).toThrowError(new Error("Cannot read FieldOrdinal, field_code out of range"))
+  })
+  test("Field throws when both type and field code out of range", () => {
+    const parser = makeParser("000101");
+    expect(() => parser.readFieldOrdinal()).toThrowError(new Error("Cannot read FieldOrdinal, type_code out of range"))
+  })
 }
-fieldParsingTests()
+
 
 function assertRecyclable (json, forField) {
   const Type = forField.associatedType
@@ -366,9 +375,12 @@ function pathSetBinaryTests () {
     )
   })
 }
-pathSetBinaryTests()
 
 describe('Binary Parser', function() {
   describe('pathSetBinaryTests', pathSetBinaryTests);
   describe('nestedObjectTests', nestedObjectTests);
+  describe('fieldParsingTests', fieldParsingTests);
+  describe('amountParsingTests', amountParsingTests);
+  describe('transactionParsingTests', transactionParsingTests);
+  describe('basicApiTests', basicApiTests);
 });
\ No newline at end of file