fix: bn.js lib accepts strings containing non-integer values and resu…

…lts in weird behavior
hirosystems · Jul 26, 2021 · da07f10 · da07f10
1 parent 912bc2f
commit da07f10
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 2 deletions.
diff --git a/packages/transactions/src/clarity/types/intCV.ts b/packages/transactions/src/clarity/types/intCV.ts
@@ -15,7 +15,7 @@ interface IntCV {
 function valueToBN(value: unknown, signed: boolean): BigNum {
   if (typeof value === 'number') {
     if (!Number.isInteger(value)) {
-      throw new TypeError(`Invalid value. Values of type 'number' must be an integer.`);
+      throw new RangeError(`Invalid value. Values of type 'number' must be an integer.`);
     }
     return new BigNum(value);
   }
@@ -28,7 +28,15 @@ function valueToBN(value: unknown, signed: boolean): BigNum {
       hex = hex.padStart(hex.length + (hex.length % 2), '0');
       value = Buffer.from(hex, 'hex');
     } else {
-      return new BigNum(value);
+      const bn = new BigNum(value);
+      // Prevent bn.js from accepting non-integer input and doing strange things. For example:
+      // `new BigNum('3.1415').toString() === '281415'`
+      if (bn.toString() !== value) {
+        throw new RangeError(
+          `Invalid value. String integer '${value}' is parsed as '${bn.toString()}'.`
+        );
+      }
+      return bn;
     }
   }
   if (typeof value === 'bigint') {

diff --git a/packages/transactions/tests/clarity.test.ts b/packages/transactions/tests/clarity.test.ts
@@ -210,6 +210,11 @@ describe('Clarity Types', () => {
         // @ts-expect-error ts(2322) Type 'string' is not assignable to type 'number'
         intCV(['example string array']);
       }).toThrowError(TypeError);
+
+      expect(() => intCV(NaN)).toThrowError(RangeError);
+      expect(() => intCV(Infinity)).toThrowError(RangeError);
+      expect(() => intCV(3.1415)).toThrowError(RangeError);
+      expect(() => intCV('3.1415')).toThrowError(RangeError);
     });
 
     test.each(
@@ -231,8 +236,10 @@ describe('Clarity Types', () => {
         [-200, '-200', '0xffffffffffffffffffffffffffffff38'],
         [Buffer.from([0xff, 0x38]), '-200', '0xffffffffffffffffffffffffffffff38'],
         [200, '200', '0x000000000000000000000000000000c8'],
+        [2e2, '200', '0x000000000000000000000000000000c8'],
         [10, '10', '0x0000000000000000000000000000000a'],
         [10n, '10', '0x0000000000000000000000000000000a'],
+        [1e1, '10', '0x0000000000000000000000000000000a'],
         ['10', '10', '0x0000000000000000000000000000000a'],
         ['0x0a', '10', '0x0000000000000000000000000000000a'],
         ['0x000a', '10', '0x0000000000000000000000000000000a'],