Skip to content

IOU amount from JSON wraps above INT64_MAX, and a negative value is accepted (Version: 3.3.0) #8188

Description

@markneonin

Issue Description

When an IOU amount is parsed from JSON, the digits of the value string are concatenated into a uint64_t, then narrowed by an unchecked static_cast<std::int64_t>. A digit string above INT64_MAX wraps, and the amount becomes a different number. If the string carries a minus sign, iou() negates the wrapped value back into positive range, the sign check that would have rejected it passes, and the server accepts the transaction.

So a trust line limit of "-1000" is rejected, and "-1000.0000000000000000", the same number with trailing zeros, is accepted as a limit of 844.6744073709552. What the value means does not matter, only the digit string it is written with.

Steps to Reproduce

simulate on Testnet. No local build, no signing, no funded account. Only LimitAmount.value changes.

curl -s -X POST https://s.altnet.rippletest.net:51234/ -H "Content-Type: application/json" -d '{
  "method": "simulate",
  "params": [{"tx_json": {
    "TransactionType": "TrustSet",
    "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
    "LimitAmount": {"currency": "USD",
                    "issuer": "ra5nK24KXen9AHvsdFTKHSANinZseWnPcX",
                    "value": "-1000.0000000000000000"}
  }}]
}'

Expected Result

"-1000.0000000000000000" is -1000, and -1000 is rejected with temBAD_LIMIT. A string differing only in trailing zeros should reach the same verdict. Digits beyond what the mantissa holds are already handled by rounding elsewhere: "12345678901234567" is accepted and canonicalized to 1234567890123457e1.

Actual Result

Testnet and mainnet, 3.3.0, identical on both:

LimitAmount.value engine_result limit echoed back
"1000" tesSUCCESS 1000
"1000.0000000000000000" temBAD_LIMIT -844.6744073709552
"-1000" temBAD_LIMIT -1000
"-1000.0000000000000000" tesSUCCESS 844.6744073709552

"1000.0000000000000000" gives the digit string 10000000000000000000, and 10^19 - 2^64 = -8446744073709551616, which at exponent -16, rounded to the 16 significant digits STAmount keeps, prints as -844.6744073709552. The minus sign then flips it positive.

The defect is in the shared JSON path, not in one transactor, so Payment.Amount behaves the same. Engine results there depend on paths and liquidity, so only the parsed amount is meaningful:

Amount.value amount echoed back
"9223.000000000000000" 9223
"9224.000000000000000" -9222.744073709552
"-1.0000000000000000000" 0.8446744073709552

Past UINT64_MAX the value is rejected instead: "44730.000000000000000" returns invalidParams (31), because boost::lexical_cast throws and the catch-all around amountFromJson reports invalid data.

Root Cause

partsFromString returns an unsigned mantissa and a separate sign, unbounded (STNumber.cpp#L198), and amountFromJson passes those parts straight into the constructor (STAmount.cpp#L1036).

The constructor states the invariant and then does not enforce it for an IOU. Under MantissaScale::Small the check is an XRPL_ASSERT, a plain assert compiled out of release builds; otherwise the throw is gated on integral(), true only for XRP and MPT (STAmount.h#L358-L370):

    // value_ is uint64, but needs to fit in the range of int64
    if (Number::getMantissaScale() == MantissaRange::MantissaScale::Small)
    {
        XRPL_ASSERT(
            value_ <= std::numeric_limits<std::int64_t>::max(),
            "xrpl::STAmount::STAmount(SField, A, std::uint64_t, int, bool) : "
            "maximum mantissa input");
    }
    else
    {
        if (integral() && value_ > std::numeric_limits<std::int64_t>::max())
            throw std::overflow_error("STAmount mantissa is too large " + std::to_string(mantissa));
    }

For an IOU, canonicalize() goes straight to iou() (STAmount.cpp#L874), where the wrap happens and line 295 turns it back positive (STAmount.cpp#L291-L295):

    auto mantissa = static_cast<std::int64_t>(value_);
    auto exponent = offset_;

    if (isNegative_)
        mantissa = -mantissa;

Suggested Fix

numberFromJson consumes the same NumberParts from the same partsFromString and handles both halves correctly: it hands the mantissa and the sign to Number separately, never casting to int64_t (STNumber.cpp#L255). STAmount::canonicalize does the same for XRP and MPT (STAmount.cpp#L840). amountFromJson is the one place that does neither.

Dropping the integral() && gate so the constructor throws would close the hole, but it rejects values the server represents exactly. The window is 19- and 20-digit strings, and the digit count is lexical, so "-1000.0000000000000000" would return invalidParams rather than parsing as -1000 and reaching temBAD_LIMIT, contrary to the Expected Result above. Ordinary large amounts fall in the same window:

LimitAmount.value limit echoed back
"9999999999999999999" -8446744073709552e3
"18446744073709551615" -1

Both are positive amounts an IOU holds without trouble, and both come back negative today. Rejecting them would also be inconsistent with the rounding the API already does: "12345678901234567" is accepted and rounded to 16 digits, so refusing a 19-digit string instead of rounding it would treat the same class of input two ways. And it would not close the hole everywhere: under MantissaScale::Small the check is an XRPL_ASSERT that release builds compile out, and amountFromString reaches a constructor with no check at all.

So the fix belongs in the parsers. When a non-integral asset's mantissa exceeds INT64_MAX, build the value through Number from the unsigned mantissa and the sign, and let STAmount::fromNumber normalize it into [kMinValue, kMaxValue], rounding away the digits an IOU cannot keep. Number::Unchecked rather than Normalized, so the result does not depend on the amendment-selected mantissa scale. amountFromString gets the same treatment.

Opened as #8190, with cases in STAmount_test.cpp::testParseJson (which currently stops at std::numeric_limits<unsigned int>::max()) and an end-to-end TrustSet case.

Scope

RPC only. JSON is not parsed in the protocol layer, so no amendment is needed, the same conclusion as #5990: "This only affects RPC because we do not parse JSON in the protocol layer, hence no amendment needed." That PR fixed a different bug in the same function.

Binary deserialization is unaffected, it checks the mantissa against kMinValue and kMaxValue at STAmount.cpp:170. Everything parsing tx_json shares the STI_AMOUNT case at STParsedJSON.cpp:679-688, so sign, submit and submit_multisigned take the same input as simulate. On a server with signing enabled, sign would sign a value the caller did not write.

I searched open and closed issues for amountFromJson, partsFromString, mantissa overflow and sign handling, and found nothing matching. #7464 is open against amountFromJson but is a different defect.

Environment

Reproduced on 2026-09-08 against Testnet (s.altnet.rippletest.net:51234) and mainnet (xrplcluster.com), both reporting 3.3.0, neither behind Clio. Source read at tag 3.3.0; every location cited is byte-identical on develop, at the same line numbers.

Supporting Files

N/A, the curl command above is the reproduction.

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