fix: resolve regex vulnerability (#1714)

* first regex fix attempt

* fix tests, cleanup

* leave 1e-7 test in

* clean up throws

* fix comments

* respond to nate's comments

src/utils/xrpConversion.ts

@@ -15,27 +15,19 @@ const SANITY_CHECK = /^-?[0-9.]+$/u
  * @throws When drops amount is invalid.
  */
 export function dropsToXrp(dropsToConvert: BigNumber.Value): string {
-  let drops = dropsToConvert
-  if (typeof drops === 'string') {
-    if (!/^-?[0-9]*\.?[0-9]*$/u.exec(drops)) {
-      throw new ValidationError(
-        `dropsToXrp: invalid value '${drops}',` +
-          ` should be a number matching (^-?[0-9]*\\.?[0-9]*$).`,
-      )
-    } else if (drops === '.') {
-      throw new ValidationError(
-        `dropsToXrp: invalid value '${drops}',` +
-          ` should be a BigNumber or string-encoded number.`,
-      )
-    }
-  }
-
   /*
    * Converting to BigNumber and then back to string should remove any
    * decimal point followed by zeros, e.g. '1.00'.
-   * Important: specify base 10 to avoid exponential notation, e.g. '1e-7'.
+   * Important: specify base BASE_10 to avoid exponential notation, e.g. '1e-7'.
    */
-  drops = new BigNumber(drops).toString(BASE_TEN)
+  const drops = new BigNumber(dropsToConvert).toString(BASE_TEN)
+
+  // check that the value is valid and actually a number
+  if (typeof dropsToConvert === 'string' && drops === 'NaN') {
+    throw new ValidationError(
+      `dropsToXrp: invalid value '${dropsToConvert}', should be a BigNumber or string-encoded number.`,
+    )
+  }
 
   // drops are only whole units
   if (drops.includes('.')) {
@@ -68,21 +60,16 @@ export function dropsToXrp(dropsToConvert: BigNumber.Value): string {
  * @throws When amount in xrp is invalid.
  */
 export function xrpToDrops(xrpToConvert: BigNumber.Value): string {
-  let xrp = xrpToConvert
-  if (typeof xrp === 'string') {
-    if (!/^-?[0-9]*\.?[0-9]*$/u.exec(xrp)) {
-      throw new ValidationError(
-        `xrpToDrops: invalid value '${xrp}', should be a number matching (^-?[0-9]*\\.?[0-9]*$).`,
-      )
-    } else if (xrp === '.') {
-      throw new ValidationError(
-        `xrpToDrops: invalid value '${xrp}', should be a BigNumber or string-encoded number.`,
-      )
-    }
+  // Important: specify base BASE_TEN to avoid exponential notation, e.g. '1e-7'.
+  const xrp = new BigNumber(xrpToConvert).toString(BASE_TEN)
+
+  // check that the value is valid and actually a number
+  if (typeof xrpToConvert === 'string' && xrp === 'NaN') {
+    throw new ValidationError(
+      `xrpToDrops: invalid value '${xrpToConvert}', should be a BigNumber or string-encoded number.`,
+    )
   }
 
-  // Important: specify base 10 to avoid exponential notation, e.g. '1e-7'.
-  xrp = new BigNumber(xrp).toString(BASE_TEN)
   /*
    * This should never happen; the value has already been
    * validated above. This just ensures BigNumber did not do
@@ -90,18 +77,14 @@ export function xrpToDrops(xrpToConvert: BigNumber.Value): string {
    */
   if (!SANITY_CHECK.exec(xrp)) {
     throw new ValidationError(
-      `xrpToDrops: failed sanity check -` +
-        ` value '${xrp}',` +
-        ` does not match (^-?[0-9.]+$).`,
+      `xrpToDrops: failed sanity check - value '${xrp}', does not match (^-?[0-9.]+$).`,
     )
   }
 
   const components = xrp.split('.')
   if (components.length > 2) {
     throw new ValidationError(
-      `xrpToDrops: failed sanity check -` +
-        ` value '${xrp}' has` +
-        ` too many decimal points.`,
+      `xrpToDrops: failed sanity check - value '${xrp}' has too many decimal points.`,
     )
   }
 

test/utils/dropsToXrp.ts

@@ -84,6 +84,15 @@ describe('dropsToXrp', function () {
     assert.strictEqual(xrp, '-2', '(number) -2 million drops equals -2 XRP')
   })
 
+  it('works with scientific notation', function () {
+    const xrp = dropsToXrp('1e6')
+    assert.strictEqual(
+      xrp,
+      '1',
+      '(scientific notation string) 1e6 drops equals 1 XRP',
+    )
+  })
+
   it('throws with an amount with too many decimal places', function () {
     assert.throws(() => {
       dropsToXrp('1.2')
@@ -101,7 +110,7 @@ describe('dropsToXrp', function () {
 
     assert.throws(() => {
       dropsToXrp('1e-7')
-    }, /invalid value/u)
+    }, /decimal place/u)
 
     assert.throws(() => {
       dropsToXrp('2,0')

test/utils/xrpToDrops.ts

@@ -61,20 +61,29 @@ describe('xrpToDrops', function () {
 
   it('works with a number', function () {
     // This is not recommended. Use strings or BigNumber objects to avoid precision errors.
-    let drops = xrpToDrops(2)
+    const drops = xrpToDrops(2)
     assert.strictEqual(
       drops,
       '2000000',
       '(number) 2 XRP equals 2 million drops',
     )
-    drops = xrpToDrops(-2)
+    const drops2 = xrpToDrops(-2)
     assert.strictEqual(
-      drops,
+      drops2,
       '-2000000',
       '(number) -2 XRP equals -2 million drops',
     )
   })
 
+  it('works with scientific notation', function () {
+    const drops = xrpToDrops('1e-6')
+    assert.strictEqual(
+      drops,
+      '1',
+      '(scientific notation string) 1e-6 XRP equals 1 drop',
+    )
+  })
+
   it('throws with an amount with too many decimal places', function () {
     assert.throws(() => {
       xrpToDrops('1.1234567')
@@ -90,7 +99,7 @@ describe('xrpToDrops', function () {
     }, /invalid value/u)
     assert.throws(() => {
       xrpToDrops('1e-7')
-    }, /invalid value/u)
+    }, /decimal place/u)
     assert.throws(() => {
       xrpToDrops('2,0')
     }, /invalid value/u)