From a1a938e895f205710054df67954e0ad37e5ec3e7 Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Tue, 15 Nov 2016 18:23:47 +1000 Subject: [PATCH] Enforce proper bounds of Amounts --- packages/ripple-binary-codec/package.json | 2 +- .../ripple-binary-codec/src/types/amount.js | 71 +++++++++++++++++-- .../ripple-binary-codec/src/types/path-set.js | 5 +- .../ripple-binary-codec/src/types/st-array.js | 2 +- .../src/types/st-object.js | 5 +- .../src/types/vector-256.js | 2 +- .../ripple-binary-codec/test/amount-test.js | 21 ++++++ .../test/fixtures/data-driven-tests.json | 9 +++ 8 files changed, 103 insertions(+), 14 deletions(-) diff --git a/packages/ripple-binary-codec/package.json b/packages/ripple-binary-codec/package.json index 4eb53cac..288f7e29 100644 --- a/packages/ripple-binary-codec/package.json +++ b/packages/ripple-binary-codec/package.json @@ -40,7 +40,7 @@ "prepublish": "npm test && npm run lint && npm run compile", "test": "istanbul test _mocha", "codecov": "cat ./coverage/coverage.json | ./node_modules/codecov.io/bin/codecov.io.js", - "lint": "if ! [ -f eslintrc ]; then curl -o eslintrc 'https://raw.githubusercontent.com/ripple/javascript-style-guide/es6/eslintrc'; echo 'parser: babel-eslint' >> eslintrc; fi; eslint -c eslintrc src/*.js test/*.js examples/*.js" + "lint": "if ! [ -f eslintrc ]; then curl -o eslintrc 'https://raw.githubusercontent.com/ripple/javascript-style-guide/es6/eslintrc'; echo 'parser: babel-eslint' >> eslintrc; fi; eslint -c eslintrc src/**/*.js test/*.js examples/*.js" }, "repository": { "type": "git", diff --git a/packages/ripple-binary-codec/src/types/amount.js b/packages/ripple-binary-codec/src/types/amount.js index 4dee6b45..dd6f0d46 100644 --- a/packages/ripple-binary-codec/src/types/amount.js +++ b/packages/ripple-binary-codec/src/types/amount.js @@ -9,18 +9,58 @@ const {Currency} = require('./currency'); const {AccountID} = require('./account-id'); const {UInt64} = require('./uint-64'); +const MIN_IOU_EXPONENT = -96; +const MAX_IOU_EXPONENT = 80; +const MAX_IOU_PRECISION = 16; +const MIN_IOU_MANTISSA = '1000' + '0000' + '0000' + '0000'; // 16 digits +const MAX_IOU_MANTISSA = '9999' + '9999' + '9999' + '9999'; // .. +const MAX_IOU = new Decimal(`${MAX_IOU_MANTISSA}e${MAX_IOU_EXPONENT}`); +const MIN_IOU = new Decimal(`${MIN_IOU_MANTISSA}e${MIN_IOU_EXPONENT}`); +const DROPS_PER_XRP = new Decimal('1e6'); +const MAX_NETWORK_DROPS = new Decimal('1e17'); +const MIN_XRP = new Decimal('1e-6') +const MAX_XRP = MAX_NETWORK_DROPS.dividedBy(DROPS_PER_XRP); + +// Never use exponential form Decimal.config({ - toExpPos: 32, - toExpNeg: -32 + toExpPos: MAX_IOU_EXPONENT + MAX_IOU_PRECISION, + toExpNeg: MIN_IOU_EXPONENT - MAX_IOU_PRECISION }); +const AMOUNT_PARAMETERS_DESCRIPTION = ` +Native values must be described in drops, a million of which equal one XRP. +This must be an integer number, with the absolute value not exceeding \ +${MAX_NETWORK_DROPS} + +IOU values must have a maximum precision of ${MAX_IOU_PRECISION} significant \ +digits. They are serialized as\na canonicalised mantissa and exponent. + +The valid range for a mantissa is between ${MIN_IOU_MANTISSA} and \ +${MAX_IOU_MANTISSA} +The exponent must be >= ${MIN_IOU_EXPONENT} and <= ${MAX_IOU_EXPONENT} + +Thus the largest serializable IOU value is: +${MAX_IOU.toString()} + +And the smallest: +${MIN_IOU.toString()} +` + function isDefined(val) { return !_.isUndefined(val); } +function raiseIllegalAmountError(value) { + throw new Error(`${value.toString()} is an illegal amount\n` + + AMOUNT_PARAMETERS_DESCRIPTION); +} + const parsers = { string(str) { - return [new Decimal(str).dividedBy('1e6'), Currency.XRP]; + if (!str.match(/\d+/)) { + raiseIllegalAmountError(str); + } + return [new Decimal(str).dividedBy(DROPS_PER_XRP), Currency.XRP]; }, object(object) { assert(isDefined(object.currency), 'currency must be defined'); @@ -36,6 +76,7 @@ const Amount = makeClass({ this.value = value || new Decimal('0'); this.currency = currency || Currency.XRP; this.issuer = issuer || null; + this.assertValueIsValid(); }, mixins: SerializedType, statics: { @@ -72,10 +113,30 @@ const Amount = makeClass({ mantissa[0] &= 0x3F; const drops = new Decimal(`${sign}0x${bytesToHex(mantissa)}`); - const xrpValue = drops.dividedBy('1e6'); + const xrpValue = drops.dividedBy(DROPS_PER_XRP); return new this(xrpValue, Currency.XRP); } }, + assertValueIsValid() { + // zero is always a valid amount value + if (!this.isZero()) { + if (this.isNative()) { + const abs = this.value.abs(); + if (abs.lt(MIN_XRP) || abs.gt(MAX_XRP)) { + // value is in XRP scale, but show the value in canonical json form + raiseIllegalAmountError(this.value.times(DROPS_PER_XRP)) + } + } else { + const p = this.value.precision(); + const e = this.exponent(); + if (p > MAX_IOU_PRECISION || + e > MAX_IOU_EXPONENT || + e < MIN_IOU_EXPONENT) { + raiseIllegalAmountError(this.value) + } + } + } + }, isNative() { return this.currency.isNative(); }, @@ -90,7 +151,7 @@ const Amount = makeClass({ return this.isNative() ? -6 : this.value.e - 15; }, valueString() { - return (this.isNative() ? this.value.times('1e6') : this.value) + return (this.isNative() ? this.value.times(DROPS_PER_XRP) : this.value) .toString(); }, toBytesSink(sink) { diff --git a/packages/ripple-binary-codec/src/types/path-set.js b/packages/ripple-binary-codec/src/types/path-set.js index 0f9e3861..5a9517bd 100644 --- a/packages/ripple-binary-codec/src/types/path-set.js +++ b/packages/ripple-binary-codec/src/types/path-set.js @@ -1,4 +1,3 @@ -'use strict'; /* eslint-disable no-unused-expressions */ const makeClass = require('../utils/make-class'); @@ -94,11 +93,11 @@ const PathSet = makeClass({ }, toBytesSink(sink) { let n = 0; - this.forEach((path) => { + this.forEach(path => { if (n++ !== 0) { sink.put([PATH_SEPARATOR_BYTE]); } - path.forEach((hop) => { + path.forEach(hop => { sink.put([hop.type()]); hop.account && (hop.account.toBytesSink(sink)); hop.currency && (hop.currency.toBytesSink(sink)); diff --git a/packages/ripple-binary-codec/src/types/st-array.js b/packages/ripple-binary-codec/src/types/st-array.js index 66c9a478..5e3e6dfb 100644 --- a/packages/ripple-binary-codec/src/types/st-array.js +++ b/packages/ripple-binary-codec/src/types/st-array.js @@ -26,7 +26,7 @@ const STArray = makeClass({ } }, toJSON() { - return this.map((v) => v.toJSON()); + return this.map(v => v.toJSON()); }, toBytesSink(sink) { this.forEach(so => so.toBytesSink(sink)); diff --git a/packages/ripple-binary-codec/src/types/st-object.js b/packages/ripple-binary-codec/src/types/st-object.js index 4ac921e2..d22dcdbd 100644 --- a/packages/ripple-binary-codec/src/types/st-object.js +++ b/packages/ripple-binary-codec/src/types/st-object.js @@ -1,4 +1,3 @@ -const assert = require('assert'); const _ = require('lodash'); const makeClass = require('../utils/make-class'); const {Field} = require('../enums'); @@ -39,7 +38,7 @@ const STObject = makeClass({ } }, fieldKeys() { - return Object.keys(this).map((k) => Field[k]).filter(Boolean); + return Object.keys(this).map(k => Field[k]).filter(Boolean); }, toJSON() { // Otherwise seemingly result will have same prototype as `this` @@ -52,7 +51,7 @@ const STObject = makeClass({ const serializer = new BinarySerializer(sink); const fields = this.fieldKeys(); const sorted = _.sortBy(fields, 'ordinal'); - sorted.filter(filter).forEach((field) => { + sorted.filter(filter).forEach(field => { const value = this[field]; if (!field.isSerialized) { return; diff --git a/packages/ripple-binary-codec/src/types/vector-256.js b/packages/ripple-binary-codec/src/types/vector-256.js index a3d49891..fdbcc44f 100644 --- a/packages/ripple-binary-codec/src/types/vector-256.js +++ b/packages/ripple-binary-codec/src/types/vector-256.js @@ -23,7 +23,7 @@ const Vector256 = makeClass({ this.forEach(h => h.toBytesSink(sink)); }, toJSON() { - return this.map((hash) => hash.toJSON()); + return this.map(hash => hash.toJSON()); } }); diff --git a/packages/ripple-binary-codec/test/amount-test.js b/packages/ripple-binary-codec/test/amount-test.js index 1b1f8b08..d8f6efbd 100644 --- a/packages/ripple-binary-codec/test/amount-test.js +++ b/packages/ripple-binary-codec/test/amount-test.js @@ -1,5 +1,25 @@ +const _ = require('lodash'); const assert = require('assert-diff'); +const utils = require('./utils'); const {Amount} = require('../src/coretypes'); +const {loadFixture} = utils; +const fixtures = loadFixture('data-driven-tests.json'); + +function amountErrorTests() { + _.filter(fixtures.values_tests, {type: 'Amount'}).forEach(f => { + // We only want these with errors + if (!f.error) { + return + } + const testName = `${JSON.stringify(f.test_json)}\n\tis invalid ` + + `because: ${f.error}` + it(testName, () => { + assert.throws(() => { + Amount.from(f.test_json); + }, JSON.stringify(f.test_json)); + }); + }); +} describe('Amount', function() { it('can be parsed from', function() { @@ -18,5 +38,6 @@ describe('Amount', function() { }; assert.deepEqual(amt.toJSON(), rewritten); }); + amountErrorTests() }); diff --git a/packages/ripple-binary-codec/test/fixtures/data-driven-tests.json b/packages/ripple-binary-codec/test/fixtures/data-driven-tests.json index fb747c07..2079f2eb 100644 --- a/packages/ripple-binary-codec/test/fixtures/data-driven-tests.json +++ b/packages/ripple-binary-codec/test/fixtures/data-driven-tests.json @@ -2875,6 +2875,15 @@ "error": "value precision of 17 is greater than maximum iou precision of 16", "is_negative": false }, + { + "test_json": { + "currency": "USD", + "value": "9999999999999999000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "issuer": "rrrrrrrrrrrrrrrrrrrrBZbvji" + }, + "type": "Amount", + "error": "exponent is too large" + }, { "test_json": { "currency": "USD",