Skip to content
This repository has been archived by the owner on Nov 24, 2021. It is now read-only.

Input validation - Amount and Fee should not allow fractional XRP drops #31

Closed
intelliot opened this issue Jul 17, 2019 · 1 comment
Closed

Comments

@intelliot
Copy link
Contributor

intelliot commented Jul 17, 2019

It is possible for a user to set an incorrect Amount or Fee.

In the XRP Ledger's transaction format, an amount that is a string like "100" represents an amount in drops. The existing code does not validate that the string contains only numbers, so it accepts a value such as "100.001" and incorrectly encodes it to the equivalent of "998001". In other words, someone intending a "100.001" drop fee could end up paying 998,001 drops instead. Another example: "120.00001" becomes '119800001'.

This appears to be caused by the bignumber library (bn.js), as it is not expected (and not tested) for Amount or Fee to contain a decimal point; they're supposed to be string-encoded integers representing an amount of drops.

The docs are clear that Amount and Fee must be integers:

Unit tests are permitted to submit values of XRP (not drops) with a decimal point - for example, "1.23" meaning 1.23 XRP. All other cases should always specify XRP in drops, with no decimal point: e.g. "1230000" meaning 1.23 XRP.

~ https://xrpl.org/basic-data-types.html#specifying-currency-amounts

Aside from unit tests, all use cases must always specify XRP in drops with no decimal point.

Integer amount of XRP, in drops, to be destroyed as a cost for distributing this transaction to the network.

~ https://xrpl.org/transaction-common-fields.html

There are two root causes of the issue:

  1. The validation is too loose, requiring that the input string just contains a number anywhere in it, so 'foo 123 bar' is valid. We should require that it only contains numbers and no decimal point(s).
  2. The bn.js (bignumber) library does not work with decimals.
@ripple ripple deleted a comment from sublimator Jul 26, 2019
@intelliot intelliot changed the title Placeholder Input validation - Amount and Fee should not allow fractional XRP drops Jul 26, 2019
@intelliot
Copy link
Contributor Author

This was fixed in version 0.2.2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant