From a7b8710d107aa7eca0bbf8fdce3fc8704370327b Mon Sep 17 00:00:00 2001 From: "jinseong.cho" Date: Tue, 21 Feb 2023 02:03:09 +0900 Subject: [PATCH 1/3] fix: fail amino sign verify with special chars --- packages/amino/src/signdoc.spec.ts | 14 ++++++-- packages/amino/src/signdoc.ts | 33 +++++++++++++++++-- .../src/signingstargateclient.spec.ts | 27 +++++++++++++++ 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/packages/amino/src/signdoc.spec.ts b/packages/amino/src/signdoc.spec.ts index 121b3587..0ae5075d 100644 --- a/packages/amino/src/signdoc.spec.ts +++ b/packages/amino/src/signdoc.spec.ts @@ -1,8 +1,8 @@ /* eslint-disable @typescript-eslint/naming-convention */ import { Random } from "@cosmjs/crypto"; -import { toBech32 } from "@cosmjs/encoding"; +import { fromUtf8, toBech32, toUtf8 } from "@cosmjs/encoding"; -import { AminoMsg, makeSignDoc, sortedJsonStringify } from "./signdoc"; +import { AminoMsg, escapeCharacters, makeSignDoc, sortedJsonStringify } from "./signdoc"; function makeRandomAddress(): string { return toBech32("cosmos", Random.getBytes(20)); @@ -132,4 +132,14 @@ describe("encoding", () => { }); }); }); + + describe("escape characters after utf8 encoding", () => { + it("works", () => { + const test = JSON.stringify({ memo: "ampersand:&,lt:<,gt:>" }); + const utf8Encoding = toUtf8(test); + const escapedEncoding = escapeCharacters(utf8Encoding); + + expect(JSON.parse(fromUtf8(utf8Encoding))).toEqual(JSON.parse(fromUtf8(escapedEncoding))); + }); + }); }); diff --git a/packages/amino/src/signdoc.ts b/packages/amino/src/signdoc.ts index 2348353e..d4417076 100644 --- a/packages/amino/src/signdoc.ts +++ b/packages/amino/src/signdoc.ts @@ -72,6 +72,35 @@ export function makeSignDoc( }; } -export function serializeSignDoc(signDoc: StdSignDoc): Uint8Array { - return toUtf8(sortedJsonStringify(signDoc)); +export function escapeCharacters(encodedArray: Uint8Array): Uint8Array { + const AmpersandUnicode = new Uint8Array([92, 117, 48, 48, 50, 54]); + const LtSignUnicode = new Uint8Array([92, 117, 48, 48, 51, 99]); + const GtSignUnicode = new Uint8Array([92, 117, 48, 48, 51, 101]); + + const AmpersandAscii = 38; + const LtSign = 60; // < + const GtSign = 62; // > + + const filteredIndex: number[] = []; + encodedArray.forEach((value, index) => { + if (value === AmpersandAscii || value === LtSign || value === GtSign) filteredIndex.push(index); + }); + + let result = new Uint8Array([...encodedArray]); + const reversedFilteredIndex = filteredIndex.reverse(); + reversedFilteredIndex.forEach((value) => { + let unicode = AmpersandUnicode; + if (result[value] === LtSign) { + unicode = LtSignUnicode; + } else if (result[value] === GtSign) { + unicode = GtSignUnicode; + } + result = new Uint8Array([...result.slice(0, value), ...unicode, ...result.slice(value + 1)]); + }); + + return result; +} + +export function serializeSignDoc(signDoc: StdSignDoc): Uint8Array { + return escapeCharacters(toUtf8(sortedJsonStringify(signDoc))); } diff --git a/packages/stargate/src/signingstargateclient.spec.ts b/packages/stargate/src/signingstargateclient.spec.ts index e0a58e2b..3f15b5c9 100644 --- a/packages/stargate/src/signingstargateclient.spec.ts +++ b/packages/stargate/src/signingstargateclient.spec.ts @@ -455,6 +455,33 @@ describe("SigningStargateClient", () => { }); describe("legacy Amino mode", () => { + it("works with special characters in memo", async () => { + pendingWithoutSimapp(); + const wallet = await Secp256k1HdWallet.fromMnemonic(faucet.mnemonic); + const client = await SigningStargateClient.connectWithSigner( + simapp.tendermintUrl, + wallet, + defaultSigningClientOptions, + ); + + const msgSend: MsgSend = { + fromAddress: faucet.address0, + toAddress: makeRandomAddress(), + amount: coins(1234, "ucosm"), + }; + const msgAny: MsgSendEncodeObject = { + typeUrl: "/cosmos.bank.v1beta1.MsgSend", + value: msgSend, + }; + const fee = { + amount: coins(2000, "ucosm"), + gas: "200000", + }; + const memo = "ampersand:&,lt:<,gt:>"; + const result = await client.signAndBroadcast(faucet.address0, [msgAny], fee, memo); + assertIsDeliverTxSuccess(result); + }); + it("works with bank MsgSend", async () => { pendingWithoutSimapp(); const wallet = await Secp256k1HdWallet.fromMnemonic(faucet.mnemonic); From 2c4e9178f9dbc1e7340890e39119014c151dfa4c Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Thu, 9 Mar 2023 18:14:44 +0100 Subject: [PATCH 2/3] Migrate to string-based escaping implementation --- packages/amino/src/signdoc.spec.ts | 41 +++++++++++++++++++++---- packages/amino/src/signdoc.ts | 49 +++++++++++++----------------- 2 files changed, 56 insertions(+), 34 deletions(-) diff --git a/packages/amino/src/signdoc.spec.ts b/packages/amino/src/signdoc.spec.ts index 0ae5075d..e4c1261c 100644 --- a/packages/amino/src/signdoc.spec.ts +++ b/packages/amino/src/signdoc.spec.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/naming-convention */ import { Random } from "@cosmjs/crypto"; -import { fromUtf8, toBech32, toUtf8 } from "@cosmjs/encoding"; +import { toBech32 } from "@cosmjs/encoding"; import { AminoMsg, escapeCharacters, makeSignDoc, sortedJsonStringify } from "./signdoc"; @@ -133,13 +133,42 @@ describe("encoding", () => { }); }); - describe("escape characters after utf8 encoding", () => { + describe("escapeCharacters", () => { it("works", () => { - const test = JSON.stringify({ memo: "ampersand:&,lt:<,gt:>" }); - const utf8Encoding = toUtf8(test); - const escapedEncoding = escapeCharacters(utf8Encoding); + // Unchanged originals + expect(escapeCharacters(`""`)).toEqual(`""`); + expect(escapeCharacters(`{}`)).toEqual(`{}`); + expect(escapeCharacters(`[]`)).toEqual(`[]`); + expect(escapeCharacters(`[123,null,"foo",[{}]]`)).toEqual(`[123,null,"foo",[{}]]`); + expect(escapeCharacters(`{"num":123}`)).toEqual(`{"num":123}`); + expect(escapeCharacters(`{"memo":"123"}`)).toEqual(`{"memo":"123"}`); + expect(escapeCharacters(`{"memo":"\\u0026"}`)).toEqual(`{"memo":"\\u0026"}`); - expect(JSON.parse(fromUtf8(utf8Encoding))).toEqual(JSON.parse(fromUtf8(escapedEncoding))); + // Escapes one + expect(escapeCharacters(`{"m":"with amp: &"}`)).toEqual(`{"m":"with amp: \\u0026"}`); + expect(escapeCharacters(`{"m":"with lt: <"}`)).toEqual(`{"m":"with lt: \\u003c"}`); + expect(escapeCharacters(`{"m":"with gt: >"}`)).toEqual(`{"m":"with gt: \\u003e"}`); + // Escapes multiple + expect(escapeCharacters(`{"m":"with amp: &&"}`)).toEqual(`{"m":"with amp: \\u0026\\u0026"}`); + expect(escapeCharacters(`{"m":"with lt: <<"}`)).toEqual(`{"m":"with lt: \\u003c\\u003c"}`); + expect(escapeCharacters(`{"m":"with gt: >>"}`)).toEqual(`{"m":"with gt: \\u003e\\u003e"}`); + expect(escapeCharacters(`{"m":"with all: &<>"}`)).toEqual(`{"m":"with all: \\u0026\\u003c\\u003e"}`); + }); + + it("escaped encoding can be decoded to the same document", () => { + const docs = [ + { memo: "ampersand:&,lt:<,gt:>", value: 123.421 }, + "", + 123, + ["foo", "ampersand:&,lt:<,gt:>"], + ]; + + for (const doc of docs) { + const normalEncoding = JSON.stringify(doc); + const escapedEncoding = escapeCharacters(normalEncoding); + expect(JSON.parse(escapedEncoding)).toEqual(JSON.parse(normalEncoding)); + expect(JSON.parse(escapedEncoding)).toEqual(doc); + } }); }); }); diff --git a/packages/amino/src/signdoc.ts b/packages/amino/src/signdoc.ts index d4417076..3a6f56c1 100644 --- a/packages/amino/src/signdoc.ts +++ b/packages/amino/src/signdoc.ts @@ -72,35 +72,28 @@ export function makeSignDoc( }; } -export function escapeCharacters(encodedArray: Uint8Array): Uint8Array { - const AmpersandUnicode = new Uint8Array([92, 117, 48, 48, 50, 54]); - const LtSignUnicode = new Uint8Array([92, 117, 48, 48, 51, 99]); - const GtSignUnicode = new Uint8Array([92, 117, 48, 48, 51, 101]); - - const AmpersandAscii = 38; - const LtSign = 60; // < - const GtSign = 62; // > - - const filteredIndex: number[] = []; - encodedArray.forEach((value, index) => { - if (value === AmpersandAscii || value === LtSign || value === GtSign) filteredIndex.push(index); - }); - - let result = new Uint8Array([...encodedArray]); - const reversedFilteredIndex = filteredIndex.reverse(); - reversedFilteredIndex.forEach((value) => { - let unicode = AmpersandUnicode; - if (result[value] === LtSign) { - unicode = LtSignUnicode; - } else if (result[value] === GtSign) { - unicode = GtSignUnicode; - } - result = new Uint8Array([...result.slice(0, value), ...unicode, ...result.slice(value + 1)]); - }); - - return result; +/** + * Takes a valid JSON document and performs the following escapings in string values: + * + * `&` -> `\u0026` + * `<` -> `\u003c` + * `>` -> `\u003e` + * + * Since those characters do not occur in other places of the JSON document, only + * string values are affected. + * + * If the input is invalid JSON, the behaviour is undefined. + */ +export function escapeCharacters(input: string): string { + // When we migrate to target es2021 or above, we can use replaceAll instead of global patterns. + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll + const amp = /&/g; + const lt = //g; + return input.replace(amp, "\\u0026").replace(lt, "\\u003c").replace(gt, "\\u003e"); } export function serializeSignDoc(signDoc: StdSignDoc): Uint8Array { - return escapeCharacters(toUtf8(sortedJsonStringify(signDoc))); + const serialized = escapeCharacters(sortedJsonStringify(signDoc)); + return toUtf8(serialized); } From 37944e8f45a8a42f29047328f1b8aafa3bd114e9 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Thu, 9 Mar 2023 18:19:22 +0100 Subject: [PATCH 3/3] Add CHANGELOG entry for Amino JSON escaping of &, < and > --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 997e74f8..545259e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ and this project adheres to ## [Unreleased] +### Fixed + +- @cosmjs/amino: Fix escaping of "&", "<" and ">" characters in Amino JSON + encoding to match the Go implementation ([#1373], [#1388]). + +[#1373]: https://github.com/cosmos/cosmjs/pull/1373 +[#1388]: https://github.com/cosmos/cosmjs/pull/1388 + ## [0.30.0] - 2023-03-09 ### Changed