From a07c2d95458b82456bc5a146b12cee816e20202d Mon Sep 17 00:00:00 2001 From: ayushsingh82 Date: Fri, 4 Sep 2026 18:31:07 +0530 Subject: [PATCH] fix: reject malformed numeric CLI arguments instead of crashing or coercing to 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit asBigInt (deposit ids, intent amounts, maxCost, durations) threw a raw SyntaxError/RangeError on a hex, fractional, or non-numeric string and on a non-integer number, and silently returned 0n for "" — so a blank --deposit-id resolved to deposit 0. It now returns a VALIDATION_ERROR and accepts only a plain decimal integer string or a safe-integer number. amountToUnits rounded a sub-base-unit amount to 0n (turning a transfer or approve into a no-op) and handed exponential notation ('1e-7', '1e+21') straight to parseUnits, which throws. Both are now rejected with a clear error. --- CHANGELOG.md | 10 ++++++++++ src/utils/parsing.ts | 27 ++++++++++++++++++++++++++- src/utils/validation.ts | 19 ++++++++++++++++++- tests/full-coverage.test.ts | 18 ++++++++++++++++++ tests/utils.test.ts | 11 +++++++++++ 5 files changed, 83 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ab02e4..c40c027 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## Unreleased + +- Hardened numeric argument parsing. `asBigInt` (deposit ids, intent amounts, + `maxCost`, durations) now rejects a blank, hex, or fractional string and a + non-integer or precision-losing number with a `VALIDATION_ERROR` instead of + throwing a raw `SyntaxError`/`RangeError` or, for `""`, silently resolving to + `0n`. `amountToUnits` rejects a sub-base-unit amount (previously rounded to + `0n`, turning a transfer into a no-op) and an amount large enough to force + exponential notation past `parseUnits`. + ## 0.3.1 - Made free-form MCP inputs emit explicit recursive JSON schemas so strict MCP diff --git a/src/utils/parsing.ts b/src/utils/parsing.ts index 0d39416..c43add3 100644 --- a/src/utils/parsing.ts +++ b/src/utils/parsing.ts @@ -5,9 +5,34 @@ export function asBigInt(value: unknown, field: string): bigint { if (typeof value === 'bigint') { return value; } - if (typeof value === 'number' || typeof value === 'string') { + if (typeof value === 'number') { + if (!Number.isInteger(value)) { + throw createError('VALIDATION_ERROR', `${field} must be a whole number, got ${value}.`); + } + if (!Number.isSafeInteger(value)) { + // Past 2^53 a JS number cannot represent the integer exactly and + // BigInt(value) would carry the rounding error through. Callers with a + // value this large must pass it as a string. + throw createError( + 'VALIDATION_ERROR', + `${field} is too large to pass as a number without losing precision; pass it as a string.`, + ); + } return BigInt(value); } + if (typeof value === 'string') { + // BigInt('') is 0n and BigInt('0x10') is 16 — accept only a plain decimal + // integer so a blank or malformed argument fails loudly instead of silently + // resolving to the wrong id / amount. + const trimmed = value.trim(); + if (!/^-?\d+$/.test(trimmed)) { + throw createError( + 'VALIDATION_ERROR', + `${field} must be an integer, got ${JSON.stringify(value)}.`, + ); + } + return BigInt(trimmed); + } throw createError('VALIDATION_ERROR', `${field} must be a bigint-compatible value.`); } diff --git a/src/utils/validation.ts b/src/utils/validation.ts index 16a312b..1a87ae4 100644 --- a/src/utils/validation.ts +++ b/src/utils/validation.ts @@ -118,7 +118,24 @@ export function ensureSupportedPlatformList(values: string[] | undefined, fieldN export function amountToUnits(value: unknown, fieldName: string, decimals = 6): bigint { const parsed = ensurePositiveNumber(value, fieldName); - return parseUnits(parsed.toString(), decimals); + const minUnit = 1 / 10 ** decimals; + if (parsed < minUnit) { + // parseUnits would round this to 0n, silently turning a nonzero request into + // a no-op transfer/approve. + throw createError('VALIDATION_ERROR', `${fieldName} must be at least ${minUnit}, got ${value}.`, { + details: { value, minimum: minUnit }, + }); + } + if (parsed >= 1e21) { + // Number#toFixed and Number#toString both switch to exponential notation at + // 1e21, which parseUnits rejects; an amount this large is not a real input. + throw createError('VALIDATION_ERROR', `${fieldName} is implausibly large: ${value}.`, { + details: { value }, + }); + } + // toFixed keeps plain decimal notation (toString emits '1e-7' for small + // magnitudes, which parseUnits throws on) and caps the fraction at `decimals`. + return parseUnits(parsed.toFixed(decimals), decimals); } export function optionalAmountToUnits(value: unknown, fieldName: string, decimals = 6): bigint | undefined { diff --git a/tests/full-coverage.test.ts b/tests/full-coverage.test.ts index e9ca43a..60f33e5 100644 --- a/tests/full-coverage.test.ts +++ b/tests/full-coverage.test.ts @@ -112,6 +112,24 @@ describe('parsing coverage', () => { expect(() => asBigInt({}, 'field')).toThrow('field must be a bigint-compatible value.'); }); + it('asBigInt accepts integer numbers and decimal-integer strings', () => { + expect(asBigInt(42, 'field')).toBe(42n); + expect(asBigInt('42', 'field')).toBe(42n); + expect(asBigInt(' -7 ', 'field')).toBe(-7n); + }); + + it('asBigInt rejects malformed strings and numbers instead of throwing raw or returning 0n', () => { + // BigInt('') is 0n and BigInt('0x10') is 16 — both must be rejected loudly. + expect(() => asBigInt('', 'depositId')).toThrow('depositId must be an integer'); + expect(() => asBigInt('0x10', 'depositId')).toThrow('depositId must be an integer'); + expect(() => asBigInt('1.5', 'depositId')).toThrow('depositId must be an integer'); + expect(() => asBigInt('abc', 'depositId')).toThrow('depositId must be an integer'); + // Non-integer / precision-losing numbers. + expect(() => asBigInt(1.5, 'maxCost')).toThrow('maxCost must be a whole number'); + expect(() => asBigInt(Number.NaN, 'maxCost')).toThrow('maxCost must be a whole number'); + expect(() => asBigInt(2 ** 53, 'maxCost')).toThrow('too large to pass as a number'); + }); + it('parseJsonObject accepts object values directly', () => { const obj = { key: 'value' }; expect(parseJsonObject(obj, 'field')).toBe(obj); diff --git a/tests/utils.test.ts b/tests/utils.test.ts index eef05ae..63f407c 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -66,6 +66,17 @@ describe('validation utils', () => { expect(ensureHexPrivateKey('0x59c6995e998f97a5a0044966f0945383f0d7d1f5eb53d3d16c23f0a3077ec12e')).toMatch(/^0x/); }); + it('amountToUnits handles sub-unit and exponential-notation amounts instead of throwing raw or silently rounding to zero', () => { + // Would stringify to '1e-7' and make parseUnits throw. + expect(() => amountToUnits(0.0000001, 'amount', 6)).toThrow('amount must be at least'); + // Would round to 0n inside parseUnits. + expect(() => amountToUnits('0.0000004', 'amount', 6)).toThrow('amount must be at least'); + // Would stringify to '1e+21'. + expect(() => amountToUnits(1e21, 'amount', 6)).toThrow('implausibly large'); + // A value at exactly one base unit still works. + expect(amountToUnits(0.000001, 'amount', 6)).toBe(1n); + }); + it('rejects invalid values with helpful errors', () => { expect(() => ensureString(' ', 'field')).toThrow('field must be a non-empty string.'); expect(() => ensureAddress('0x123', 'field')).toThrow('field must be a valid EVM address.');