Handle edge cases for standard currency code signing (#2009)

* Allow decoding symbols + lowercase standard codes
* Standardize the treatment of standard codes and hex codes for verifying transaction equivalence
This commit is contained in:
Jackson Mills
2022-06-17 14:27:47 -07:00
committed by GitHub
parent 7732f22858
commit 89240eae62
6 changed files with 180 additions and 10 deletions

View File

@@ -1,6 +1,7 @@
# ripple-binary-codec Release History
## Unreleased
- Fixed standard currency codes with lowercase and allowed symbols not decoding into standard codes.
## 1.4.1 (2022-06-02)
- Added a clearer error message for trying to encode an invalid transaction. (Ex. With an incorrect TransactionType)

View File

@@ -2,7 +2,7 @@ import { Hash160 } from './hash-160'
import { Buffer } from 'buffer/'
const XRP_HEX_REGEX = /^0{40}$/
const ISO_REGEX = /^[A-Z0-9]{3}$/
const ISO_REGEX = /^[A-Z0-9a-z?!@#$%^&*(){}[\]|]{3}$/
const HEX_REGEX = /^[A-F0-9]{40}$/
// eslint-disable-next-line no-control-regex
const STANDARD_FORMAT_HEX_REGEX = /^0{24}[\x00-\x7F]{6}0{10}$/

View File

@@ -54,15 +54,19 @@ describe('Currency', function () {
const currencyCode = '0000000000000000000000005852500000000000'
expect(Currency.from(currencyCode).toJSON()).toBe(currencyCode)
})
test('Currency with lowercase letters decode to hex', () => {
expect(Currency.from('xRp').toJSON()).toBe(
'0000000000000000000000007852700000000000',
test('Currency code with lowercase letters decodes to ISO code', () => {
expect(Currency.from('xRp').toJSON()).toBe('xRp')
})
test('Currency codes with symbols decodes to ISO code', () => {
expect(Currency.from('x|p').toJSON()).toBe('x|p')
})
test('Currency code with non-standard symbols decodes to hex', () => {
expect(Currency.from(':::').toJSON()).toBe(
'0000000000000000000000003A3A3A0000000000',
)
})
test('Currency codes with symbols decode to hex', () => {
expect(Currency.from('x|p').toJSON()).toBe(
'000000000000000000000000787C700000000000',
)
test('Currency codes can be exclusively standard symbols', () => {
expect(Currency.from('![]').toJSON()).toBe('![]')
})
test('Currency codes with uppercase and 0-9 decode to ISO codes', () => {
expect(Currency.from('X8P').toJSON()).toBe('X8P')

View File

@@ -3,6 +3,9 @@
Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xrpl-announce) for release announcements. We recommend that xrpl.js (ripple-lib) users stay up-to-date with the latest stable release.
## Unreleased
### Fixed
* Signing tx with standard currency codes with lowercase and allowed symbols causing an error on decode.
### Added
* When connected to nft-devnet, Client.fundWallet now defaults to using the nft-devnet faucet instead of requiring specification.

View File

@@ -24,8 +24,10 @@ import {
} from 'ripple-keypairs'
import ECDSA from '../ECDSA'
import { ValidationError } from '../errors'
import { ValidationError, XrplError } from '../errors'
import { IssuedCurrencyAmount } from '../models/common'
import { Transaction } from '../models/transactions'
import { isIssuedCurrency } from '../models/transactions/common'
import { isHex } from '../models/utils'
import { ensureClassicAddress } from '../sugar/utils'
import { hashSignedTx } from '../utils/hashes/hashLedger'
@@ -303,6 +305,7 @@ class Wallet {
* @param multisign - Specify true/false to use multisign or actual address (classic/x-address) to make multisign tx request.
* @returns A signed transaction.
* @throws ValidationError if the transaction is already signed or does not encode/decode to same result.
* @throws XrplError if the issued currency being signed is XRP ignoring case.
*/
// eslint-disable-next-line max-lines-per-function -- introduced more checks to support both string and boolean inputs.
public sign(
@@ -351,6 +354,7 @@ class Wallet {
this.privateKey,
)
}
const serialized = encode(txToSignAndEncode)
this.checkTxSerialization(serialized, tx)
return {
@@ -392,6 +396,7 @@ class Wallet {
* @param tx - The transaction prior to signing.
* @throws A ValidationError if the transaction does not have a TxnSignature/Signers property, or if
* the serialized Transaction desn't match the original transaction.
* @throws XrplError if the transaction includes an issued currency which is equivalent to XRP ignoring case.
*/
// eslint-disable-next-line class-methods-use-this, max-lines-per-function -- Helper for organization purposes
private checkTxSerialization(serialized: string, tx: Transaction): void {
@@ -449,6 +454,38 @@ class Wallet {
txCopy.URI = txCopy.URI.toUpperCase()
}
/* eslint-disable @typescript-eslint/consistent-type-assertions -- We check at runtime that this is safe */
Object.keys(txCopy).forEach((key) => {
const standard_currency_code_len = 3
if (txCopy[key] && isIssuedCurrency(txCopy[key])) {
const decodedAmount = decoded[key] as unknown as IssuedCurrencyAmount
const decodedCurrency = decodedAmount.currency
const txCurrency = (txCopy[key] as IssuedCurrencyAmount).currency
if (
txCurrency.length === standard_currency_code_len &&
txCurrency.toUpperCase() === 'XRP'
) {
throw new XrplError(
`Trying to sign an issued currency with a similar standard code to XRP (received '${txCurrency}'). XRP is not an issued currency.`,
)
}
// Standardize the format of currency codes to the 40 byte hex string for comparison
const amount = txCopy[key] as IssuedCurrencyAmount
if (amount.currency.length !== decodedCurrency.length) {
/* eslint-disable-next-line max-depth -- Easier to read with two if-statements */
if (decodedCurrency.length === standard_currency_code_len) {
decodedAmount.currency = isoToHex(decodedCurrency)
} else {
/* eslint-disable-next-line @typescript-eslint/no-unsafe-member-access -- We need to update txCopy directly */
txCopy[key].currency = isoToHex(txCopy[key].currency)
}
}
}
})
/* eslint-enable @typescript-eslint/consistent-type-assertions -- Done with dynamic checking */
if (!_.isEqual(decoded, txCopy)) {
const data = {
decoded,
@@ -509,4 +546,20 @@ function removeTrailingZeros(tx: Transaction): void {
}
}
/**
* Convert an ISO code to a hex string representation
*
* @param iso - A 3 letter standard currency code
*/
/* eslint-disable @typescript-eslint/no-magic-numbers -- Magic numbers are from rippleds of currency code encoding */
function isoToHex(iso: string): string {
const bytes = Buffer.alloc(20)
if (iso !== 'XRP') {
const isoBytes = iso.split('').map((chr) => chr.charCodeAt(0))
bytes.set(isoBytes, 12)
}
return bytes.toString('hex').toUpperCase()
}
/* eslint-enable @typescript-eslint/no-magic-numbers -- Only needed in this function */
export default Wallet

View File

@@ -1,6 +1,6 @@
import { assert } from 'chai'
import { decode } from 'ripple-binary-codec/dist'
import { NFTokenMint, Transaction } from 'xrpl-local'
import { NFTokenMint, Payment, Transaction } from 'xrpl-local'
import ECDSA from 'xrpl-local/ECDSA'
import Wallet from 'xrpl-local/Wallet'
@@ -301,6 +301,7 @@ describe('Wallet', function () {
})
})
// eslint-disable-next-line max-statements -- Required for test coverage.
describe('sign', function () {
let wallet: Wallet
@@ -590,6 +591,114 @@ describe('Wallet', function () {
}, /^1.1234567 is an illegal amount/u)
})
const issuedCurrencyPayment: Transaction = {
TransactionType: 'Payment',
Account: 'r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59',
Destination: 'rQ3PTWGLCbPz8ZCicV5tCX3xuymojTng5r',
Amount: {
currency: 'foo',
issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
value: '123.40',
},
Flags: 2147483648,
Sequence: 23,
LastLedgerSequence: 8819954,
Fee: '12',
}
it('lowercase standard currency code signs successfully', async function () {
const payment: Payment = { ...issuedCurrencyPayment }
payment.Amount = {
currency: 'foo',
issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
value: '123.40',
}
assert.deepEqual(wallet.sign(payment), {
tx_blob:
'12000022800000002400000017201B008694F261D504625103A72000000000000000000000000000666F6F00000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F54474473045022100D32EBD44F86FB6D0BE239A410B62A73A8B0C26CE3767321913D6FB7BE6FAC2410220430C011C25091DA9CD75E7C99BE406572FBB57B92132E39B4BF873863E744E2E81145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648',
hash: 'F822EA1D7B2A3026E4654A9152896652C3843B5690F8A56C4217CB4690C5C95A',
})
})
it('issued currency in standard or hex format signs to the same transaction', async function () {
const payment: Payment = { ...issuedCurrencyPayment }
payment.Amount = {
currency: '***',
issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
value: '123.40',
}
const payment2: Payment = { ...issuedCurrencyPayment }
payment2.Amount = {
currency: '0000000000000000000000002A2A2A0000000000',
issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
value: '123.40',
}
assert.deepEqual(wallet.sign(payment), wallet.sign(payment2))
})
it('sign throws when a payment contains an issued currency like XRP', async function () {
const payment: Payment = { ...issuedCurrencyPayment }
payment.Amount = {
currency: 'xrp',
issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
value: '123.40',
}
assert.throws(() => {
wallet.sign(payment)
}, /^Trying to sign an issued currency with a similar standard code to XRP \(received 'xrp'\)\. XRP is not an issued currency\./u)
})
it('sign does NOT throw when a payment contains an issued currency like xrp in hex string format', async function () {
const payment: Payment = { ...issuedCurrencyPayment }
payment.Amount = {
currency: '0000000000000000000000007872700000000000',
issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
value: '123.40',
}
assert.deepEqual(wallet.sign(payment), {
tx_blob:
'12000022800000002400000017201B008694F261D504625103A7200000000000000000000000000078727000000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F5447446304402202CD2BE27480860765B1B8DB6C499D299734C533F4FFA66317E46D1ADE5181EB7022066D2C65B975A6A9FEE56AB55211D5F2F65D6F988C8280019211874D11771A05D81145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648',
hash: '1FEAA7894E507E36D73F60DED89852CE28994366879BC7D3D806E4C50D10B1EE',
})
})
it('sign succeeds with standard currency code with symbols', async function () {
const payment: Payment = { ...issuedCurrencyPayment }
payment.Amount = {
currency: '***',
issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
value: '123.40',
}
const result = wallet.sign(payment)
const expectedResult = {
tx_blob:
'12000022800000002400000017201B008694F261D504625103A720000000000000000000000000002A2A2A00000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F54474463044022073E71588750C3D47D7D9A541F00FB897823DA67ED198D0A74404B6FE6D5E4AB5022021BE798D4159F375EBE13D0545F50EE864DF834D5A9F9A31504212156A57934C81145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648',
hash: '95BF9931C1EA164960FE13A504D5FBAEB1E072C1D291D75B85BA3F22A50346DF',
}
assert.deepEqual(result, expectedResult)
})
it('sign succeeds with non-standard 3 digit currency code', async function () {
const payment: Payment = { ...issuedCurrencyPayment }
payment.Amount = {
currency: ':::',
issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi',
value: '123.40',
}
const result = wallet.sign(payment)
const expectedResult = {
tx_blob:
'12000022800000002400000017201B008694F261D504625103A720000000000000000000000000003A3A3A00000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F5447446304402205952993DB235D3A6398E2CB5F91D7F0AD9067F02CB8E62FD335C516B64130F4702206777746CC516F95F39ADDD62CD395AF2F6BAFCCA355B5D23B9B4D9358474A11281145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648',
hash: 'CE80072E6D70932BC7AA698B931BCF97B6CC3DD3984E08DF284B74E8CB4E543A',
}
assert.deepEqual(result, expectedResult)
})
it('sign handles non-XRP amount with a trailing zero', async function () {
const payment: Transaction = {
TransactionType: 'Payment',