Skip to content

bug: parseAmount accepts malformed amount strings with trailing characters #38

Description

@AtakanGs

Summary

parseAmount in lib/utils/amount.ts uses parseFloat() after stripping supported formatting characters. Because parseFloat() accepts a valid numeric prefix and ignores trailing invalid characters, malformed amount strings can be treated as valid amounts instead of being rejected.

Current behavior

For example:

parseAmount("10abc") // returns 10
parseAmount("1.25xyz") // returns 1.25

The current implementation is:

const amount = parseFloat(cleanAmount);

if (Number.isNaN(amount) || amount <= 0) {
throw new Error(Invalid amount: ${amountStr});
}

Since parseFloat() stops parsing when it reaches an invalid character, an input that begins with a valid number can still pass validation.

Why this matters

parseAmount() is used in escrow transaction flows, including deposit and refund handling, to parse agreement amount values.

Silently accepting malformed input can cause the application to use or record an amount that differs from the original value instead of rejecting the invalid data.

For example:

"10abc" -> 10
"1.25xyz" -> 1.25

This behavior is especially undesirable for monetary values.

Expected behavior

After the supported formatting characters are normalized or removed, the entire remaining string should represent a valid positive numeric amount.

Malformed values containing unexpected trailing or embedded characters should be rejected.

Examples that should be rejected:

10abc
1.25xyz
5USDCx

Existing supported formatting such as currency symbols, commas, whitespace, parentheses, and Unicode minus normalization should continue to work as intended.

Suggested fix

Validate the entire normalized amount string before converting it to a number instead of relying on parseFloat() alone.

It would also be useful to add regression tests covering both valid formatted monetary values and malformed values containing trailing or embedded non-numeric characters.

I can submit a small PR with the validation change and regression tests if this approach looks good.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions