search

XRPLF / xrpl.js

A JavaScript/TypeScript API for interacting with the XRP Ledger in Node.js and the browser
https://xrpl.org/
1.21k stars 512 forks source link

Signing an issued currency transaction with lowercase or symbol-containing "Standard" currency codes fails #1528

Closed mDuo13 closed 2 years ago

mDuo13 commented 3 years ago

When signing a transaction involving an issued currency code that contains lower-case letters or other allowed symbols, ripple-lib raises an error for a (de-)serialization mismatch.

Steps to Reproduce

Clone https://github.com/XRPLF/xrpl-dev-portal/tree/issue_token_tutorial/content/_code-samples/issue-a-token

(Note, to run in Node.js, you need to uncomment some "require" lines towards the beginning of the file. Or you can just open the demo.html file in your browser.)

Edit the currency code (line 123) from "FOO" to "$$$", "foo", or some other string that should be treated as 3-character "Standard" currency code but is not strictly uppercase alphanumeric.

Run the sample code (note, it depends on a submit-and-verify file in a sibling folder)

Expected Result

Transaction is signed and returned successfully, the same as if the code was "FOO" or "999".

Actual Result

Uncaught (in promise) extends: Serialized transaction does not match original txJSON. See `error.data`
    u https://unpkg.com/ripple-lib@1.9.8/build/ripple-latest-min.js:2
    <anonymous> https://unpkg.com/ripple-lib@1.9.8/build/ripple-latest-min.js:2
    g https://unpkg.com/ripple-lib@1.9.8/build/ripple-latest-min.js:2
    g https://unpkg.com/ripple-lib@1.9.8/build/ripple-latest-min.js:2
    default https://unpkg.com/ripple-lib@1.9.8/build/ripple-latest-min.js:2
    main file:///another/devel/xrpl-dev-portal/content/_code-samples/issue-a-token/issue-a-token.js:138
ripple-latest-min.js:2:21241
    main file:///another/devel/xrpl-dev-portal/content/_code-samples/issue-a-token/issue-a-token.js:191
    InterpretGeneratorResume self-hosted:1482
    AsyncFunctionNext self-hosted:692

From what I can tell, it gets (de-)serialized into the 160-bit hex equivalent of the same currency code, so that gets treated as a mismatch.

I'm not sure if it's an oversight or a security feature that ripple-lib doesn't deserialize codes containing symbols & lower-case letters into "standard" codes. (After all, codes are case-sensitive so you can issue "xrp" or "xRP" that aren't actual XRP, and you could surprise people with currency codes that contain characters like < and &. Not sure if it could be used for something like script injection on a vulnerable client site, but maybe!) If it is a security feature, then the solution probably entails figuring out if the hex and 3-character codes are equivalent before raising an error, but still displaying the hex version to users.

mvadari commented 3 years ago

I believe this is because we want to avoid people using lowercase values to trick people, such as having an issued currency called xRP or something. This is a conscious choice.

mvadari commented 3 years ago

cc @natenichols @ledhed2222 who I think I talked to about this

mDuo13 commented 3 years ago

I believe this is because we want to avoid people using lowercase values to trick people, such as having an issued currency called xRP or something. This is a conscious choice.

Then as I wrote above:

If it is a security feature, then the solution probably entails figuring out if the hex and 3-character codes are equivalent before raising an error, but still displaying the hex version to users.

intelliot commented 2 years ago

Fixed in xrpl.js 2.3.1 / ripple-binary-codec 1.4.2