diff --git a/.changeset/tame-years-perform.md b/.changeset/tame-years-perform.md new file mode 100644 index 000000000000..20e88976b7b3 --- /dev/null +++ b/.changeset/tame-years-perform.md @@ -0,0 +1,5 @@ +--- +"@ledgerhq/hw-app-eth": patch +--- + +Fix `destructTypeFromString` not splitting types correctly when they contained a number and weren't native types (Struct with numbers in the name) diff --git a/libs/ledgerjs/packages/hw-app-eth/src/modules/EIP712/index.ts b/libs/ledgerjs/packages/hw-app-eth/src/modules/EIP712/index.ts index 0a81bfb46a26..0b4d22c74223 100644 --- a/libs/ledgerjs/packages/hw-app-eth/src/modules/EIP712/index.ts +++ b/libs/ledgerjs/packages/hw-app-eth/src/modules/EIP712/index.ts @@ -124,7 +124,7 @@ const makeRecursiveFieldStructImplem = ({ value: { data, type: typeDescription?.name || "", - sizeInBits: typeDescription?.bits, + sizeInBits: typeDescription?.size, }, }); } diff --git a/libs/ledgerjs/packages/hw-app-eth/src/modules/EIP712/utils.ts b/libs/ledgerjs/packages/hw-app-eth/src/modules/EIP712/utils.ts index f579baca5e4c..450de6ba5df7 100644 --- a/libs/ledgerjs/packages/hw-app-eth/src/modules/EIP712/utils.ts +++ b/libs/ledgerjs/packages/hw-app-eth/src/modules/EIP712/utils.ts @@ -27,36 +27,36 @@ export const EIP712_TYPE_PROPERTIES: Record< string, { key: (size?: number) => number; - sizeInBits: (size?: number) => number | null; + size: (size?: number) => number | null; } > = { CUSTOM: { key: () => 0, - sizeInBits: () => null, + size: () => null, }, INT: { key: () => 1, - sizeInBits: size => Number(size) / 8, + size: size => Number(size) / 8, }, UINT: { key: () => 2, - sizeInBits: size => Number(size) / 8, + size: size => Number(size) / 8, }, ADDRESS: { key: () => 3, - sizeInBits: () => null, + size: () => null, }, BOOL: { key: () => 4, - sizeInBits: () => null, + size: () => null, }, STRING: { key: () => 5, - sizeInBits: () => null, + size: () => null, }, BYTES: { key: size => (typeof size !== "undefined" ? 6 : 7), - sizeInBits: size => (typeof size !== "undefined" ? Number(size) : null), + size: size => (typeof size !== "undefined" ? Number(size) : null), }, }; @@ -66,7 +66,7 @@ export const EIP712_TYPE_PROPERTIES: Record< * A Map of encoders to transform a value to formatted buffer */ export const EIP712_TYPE_ENCODERS = { - INT(value: string | null, sizeInBits = 256): Buffer { + INT(value: string | null, size = 256): Buffer { const failSafeValue = value ?? "0"; if (typeof failSafeValue === "string" && failSafeValue?.startsWith("0x")) { @@ -78,7 +78,7 @@ export const EIP712_TYPE_ENCODERS = { // "reversibly convert a positive binary number into a negative binary number with equivalent (but negative) value". // thx wikipedia if (valueAsBN.lt(0)) { - const sizeInBytes = sizeInBits / 8; + const sizeInBytes = size / 8; // Creates BN from a buffer serving as a mask filled by maximum value 0xff const maskAsBN = new BigNumber(`0x${Buffer.alloc(sizeInBytes, 0xff).toString("hex")}`); @@ -109,10 +109,10 @@ export const EIP712_TYPE_ENCODERS = { return Buffer.from(value ?? "", "utf-8"); }, - BYTES(value: string | null, sizeInBits?: number): Buffer { + BYTES(value: string | null, size?: number): Buffer { const failSafeValue = value ?? ""; // Why slice again ? - return hexBuffer(failSafeValue).slice(0, sizeInBits ?? (failSafeValue?.length - 2) / 2); + return hexBuffer(failSafeValue).slice(0, size ?? (failSafeValue?.length - 2) / 2); }, }; @@ -132,23 +132,23 @@ export const EIP712_TYPE_ENCODERS = { */ export const destructTypeFromString = ( typeName?: string, -): [{ name: string; bits: number | undefined } | null, Array] => { +): [{ name: string; size: number | undefined } | null, Array] => { // Will split "any[][1][10]" in "any", "[][1][10]" const splitNameAndArraysRegex = new RegExp(/^([^[\]]*)(\[.*\])*/g); // Will match all numbers (or null) inside each array. [0][10][] => [0,10,null] const splitArraysRegex = new RegExp(/\[(\d*)\]/g); - // Will separate the the name from the potential bits allocation. uint8 => [uint,8] - const splitNameAndNumberRegex = new RegExp(/(\D*)(\d*)/); + // Will separate the the name from the potential bits/bytes allocation. uint8 => [uint,8] + const splitNameAndNumberRegex = new RegExp(/(?=u?int|bytes)([a-zA-Z-0-9]+?)(\d{1,3})$/g); const [, type, maybeArrays] = splitNameAndArraysRegex.exec(typeName || "") || []; - const [, name, bits] = splitNameAndNumberRegex.exec(type || "") || []; - const typeDescription = name ? { name, bits: bits ? Number(bits) : undefined } : null; + const [, name = type, size] = splitNameAndNumberRegex.exec(type || "") || []; + const typeDescription = name ? { name, size: size ? Number(size) : undefined } : null; const arrays = maybeArrays ? [...maybeArrays.matchAll(splitArraysRegex)] : []; // Parse each size to either a Number or null - const arraySizes = arrays.map(([, size]) => (size ? Number(size) : null)); + const arrayLengths = arrays.map(([, arrayLength]) => (arrayLength ? Number(arrayLength) : null)); - return [typeDescription, arraySizes]; + return [typeDescription, arrayLengths]; }; /** @@ -200,10 +200,10 @@ export const makeTypeEntryStructBuffer = ({ name, type }: EIP712MessageTypesEntr EIP712_TYPE_PROPERTIES[typeDescription?.name?.toUpperCase() || ""] || EIP712_TYPE_PROPERTIES.CUSTOM; - const typeKey = typeProperties.key(typeDescription?.bits); - const typeSizeInBits = typeProperties.sizeInBits(typeDescription?.bits); + const typeKey = typeProperties.key(typeDescription?.size); + const typeSize = typeProperties.size(typeDescription?.size); - const typeDescData = constructTypeDescByteString(isTypeAnArray, typeSizeInBits, typeKey); + const typeDescData = constructTypeDescByteString(isTypeAnArray, typeSize, typeKey); const bufferArray: Buffer[] = [Buffer.from(typeDescData, "hex")]; @@ -212,8 +212,8 @@ export const makeTypeEntryStructBuffer = ({ name, type }: EIP712MessageTypesEntr bufferArray.push(Buffer.from(typeDescription?.name ?? "", "utf-8")); } - if (typeof typeSizeInBits === "number") { - bufferArray.push(Buffer.from(intAsHexBytes(typeSizeInBits, 1), "hex")); + if (typeof typeSize === "number") { + bufferArray.push(Buffer.from(intAsHexBytes(typeSize, 1), "hex")); } if (isTypeAnArray) { diff --git a/libs/ledgerjs/packages/hw-app-eth/tests/EIP712/utils.unit.test.ts b/libs/ledgerjs/packages/hw-app-eth/tests/EIP712/utils.unit.test.ts index 9c7dd3781b8c..2c5b3a9f344c 100644 --- a/libs/ledgerjs/packages/hw-app-eth/tests/EIP712/utils.unit.test.ts +++ b/libs/ledgerjs/packages/hw-app-eth/tests/EIP712/utils.unit.test.ts @@ -10,7 +10,6 @@ import { getPayloadForFilterV2, makeTypeEntryStructBuffer, } from "../../src/modules/EIP712/utils"; -import { sign } from "crypto"; const convertTwosComplementToDecimalString = (hex: string, initialValue: string) => { if (!initialValue?.startsWith("-")) { @@ -26,35 +25,46 @@ const convertTwosComplementToDecimalString = (hex: string, initialValue: string) describe("EIP712", () => { describe("Utils", () => { describe("destructTypeFromString", () => { - test("'string[]' should return [{name: 'string', bits: undefined}, [null]]", () => { + test("'string[]' should return [{name: 'string', size: undefined}, [null]]", () => { expect(destructTypeFromString("string[]")).toEqual([ - { name: "string", bits: undefined }, + { name: "string", size: undefined }, [null], ]); }); - test("'uint8[2][][4]' should return [{name: 'uint', bits: 8}, [2, null, 4]]", () => { + test("'uint8[2][][4]' should return [{name: 'uint', size: 8}, [2, null, 4]]", () => { expect(destructTypeFromString("uint8[2][][4]")).toEqual([ - { name: "uint", bits: 8 }, + { name: "uint", size: 8 }, [2, null, 4], ]); }); - test("'bytes64' should return [{ name: 'bytes', bits: 64 }, []]", () => { - expect(destructTypeFromString("bytes64")).toEqual([{ name: "bytes", bits: 64 }, []]); + test("'bytes64' should return [{ name: 'bytes', size: 64 }, []]", () => { + expect(destructTypeFromString("bytes64")).toEqual([{ name: "bytes", size: 64 }, []]); }); - test("'bool' should return [{ name: 'bool', bits: undefined }, []]", () => { - expect(destructTypeFromString("bool")).toEqual([{ name: "bool", bits: undefined }, []]); + test("'bool' should return [{ name: 'bool', size: undefined }, []]", () => { + expect(destructTypeFromString("bool")).toEqual([{ name: "bool", size: undefined }, []]); }); test("'bool[any]' should not throw and return ['bool', []]", () => { expect(destructTypeFromString("bool[any]")).toEqual([ - { name: "bool", bits: undefined }, + { name: "bool", size: undefined }, [], ]); }); + test("V2DutchOrder should not be splitted even though it contains a number not related to the size", () => { + expect(destructTypeFromString("V2DutchOrder")).toEqual([ + { name: "V2DutchOrder", size: undefined }, + [], + ]); + }); + + test("KvnV2 should not be splitted even though it contains a number at the end not related to the size", () => { + expect(destructTypeFromString("KvnV2")).toEqual([{ name: "KvnV2", size: undefined }, []]); + }); + test("should not throw with undefined", () => { expect(destructTypeFromString(undefined)).toEqual([null, []]); });