Skip to content

Commit

Permalink
Fix parseUints with excess zeros and fix ReDoS issue (#2016, #1975, #…
Browse files Browse the repository at this point in the history
  • Loading branch information
ricmoo committed Sep 16, 2021
1 parent f2a32d0 commit 32a6b2a
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
19 changes: 13 additions & 6 deletions packages/bignumber/src.ts/fixednumber.ts
Expand Up @@ -68,6 +68,7 @@ export function formatFixed(value: BigNumberish, decimals?: string | BigNumberis
}

export function parseFixed(value: string, decimals?: BigNumberish): BigNumber {

if (decimals == null) { decimals = 0; }
const multiplier = getMultiplier(decimals);

Expand All @@ -93,14 +94,19 @@ export function parseFixed(value: string, decimals?: BigNumberish): BigNumber {
if (!whole) { whole = "0"; }
if (!fraction) { fraction = "0"; }

// Get significant digits to check truncation for underflow
{
const sigFraction = fraction.replace(/^([0-9]*?)(0*)$/, (all, sig, zeros) => (sig));
if (sigFraction.length > multiplier.length - 1) {
throwFault("fractional component exceeds decimals", "underflow", "parseFixed");
}
// Trim trialing zeros
while (fraction[fraction.length - 1] === "0") {
fraction = fraction.substring(0, fraction.length - 1);
}

This comment has been minimized.

Copy link
@ChALkeR

ChALkeR Sep 17, 2021

This is very slow and consumes memory by creating extra strings.

This is about 10x faster:

const trimRightZeroes = fraction => {
  let len = fraction.length
  while (len > 0 && fraction[len - 1] === '0') len--
  return fraction.substring(0, len)
}

// Check the fraction doesn't exceed our decimals
if (fraction.length > multiplier.length - 1) {
throwFault("fractional component exceeds decimals", "underflow", "parseFixed");
}

// If decimals is 0, we have an empty string for fraction
if (fraction === "") { fraction = "0"; }

// Fully pad the string with zeros to get to wei
while (fraction.length < multiplier.length - 1) { fraction += "0"; }

Expand All @@ -114,6 +120,7 @@ export function parseFixed(value: string, decimals?: BigNumberish): BigNumber {
return wei;
}


export class FixedFormat {
readonly signed: boolean;
readonly width: number;
Expand Down
17 changes: 17 additions & 0 deletions packages/tests/src.ts/test-utils.ts
Expand Up @@ -223,6 +223,23 @@ describe('Test Unit Conversion', function () {
assert.equal(ethers.utils.commify(test), tests[test]);
});
});

// See #2016; @TODO: Add more tests along these lines
it("checks extra tests", function() {
assert.ok(ethers.utils.parseUnits("2", 0).eq(2), "folds trailing zeros without decimal: 2");
assert.ok(ethers.utils.parseUnits("2.", 0).eq(2), "folds trailing zeros without decimal: 2.");
assert.ok(ethers.utils.parseUnits("2.0", 0).eq(2), "folds trailing zeros without decimal: 2.0");
assert.ok(ethers.utils.parseUnits("2.00", 0).eq(2), "folds trailing zeros without decimal: 2.00");

assert.ok(ethers.utils.parseUnits("2", 1).eq(20), "folds trailing zeros: 2");
assert.ok(ethers.utils.parseUnits("2.", 1).eq(20), "folds trailing zeros: 2.");
assert.ok(ethers.utils.parseUnits("2.0", 1).eq(20), "folds trailing zeros: 2.0");
assert.ok(ethers.utils.parseUnits("2.00", 1).eq(20), "folds trailing zeros: 2.00");

assert.ok(ethers.utils.parseUnits("2.5", 1).eq(25), "folds trailing zeros: 2.5");
assert.ok(ethers.utils.parseUnits("2.50", 1).eq(25), "folds trailing zeros: 2.50");
assert.ok(ethers.utils.parseUnits("2.500", 1).eq(25), "folds trailing zeros: 2.500");
});
});


Expand Down

0 comments on commit 32a6b2a

Please sign in to comment.