Skip to content

Commit

Permalink
✨ (keyring-eth) [DSDK-530]: Handle empty arrays with filters (#450)
Browse files Browse the repository at this point in the history
  • Loading branch information
@paoun-ledger
paoun-ledger authored Nov 5, 2024
2 parents 9525b13 + a747e4b commit 62638cc
Show file tree
Hide file tree
Showing 7 changed files with 289 additions and 68 deletions.
5 changes: 5 additions & 0 deletions .changeset/hip-ducks-study.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ledgerhq/device-signer-kit-ethereum": patch
---

Fix clear signing of EIP712 messages with an empty array
15 changes: 8 additions & 7 deletions packages/signer/context-module/src/typed-data/domain/DefaultTypedDataContextLoader.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ describe("TokenContextLoader", () => {
expect(result.type).toEqual("success");
});

it("should return an error if value is not found", async () => {
it("should ignore the token if value is not found", async () => {
// GIVEN
const ctx = {
verifyingContract: "0x000000000022d473030f116ddee9f6b43ac78ba3",
Expand Down Expand Up @@ -522,12 +522,13 @@ describe("TokenContextLoader", () => {
const result = await loader.load(ctx);

// THEN
expect(result).toEqual({
type: "error",
error: new Error(
"The token filter references the value details.badtoken which is absent from the message",
),
});
expect(result.type).toEqual("success");
if (result.type === "success") {
expect(result.filters["details.badtoken"]?.["displayName"]).toEqual(
"Amount allowance",
);
expect(result.tokens).toEqual({});
}
});
});
});
8 changes: 2 additions & 6 deletions packages/signer/context-module/src/typed-data/domain/DefaultTypedDataContextLoader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,8 @@ export class DefaultTypedDataContextLoader implements TypedDataContextLoader {
(entry) => entry.path === filter.path,
);
if (values.length === 0) {
return {
type: "error",
error: new Error(
`The token filter references the value ${filter.path} which is absent from the message`,
),
};
// No value matching the referenced token. It may be located in an empty array.
continue;
}
const value = values[0]!;
const address = this.convertAddressToHexaString(value.value);
Expand Down
39 changes: 39 additions & 0 deletions ...ages/signer/signer-eth/src/internal/app-binder/command/SendEIP712FilteringCommand.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ const DATE_TIME_APDU = Uint8Array.from([
0xaf, 0x30, 0x3d, 0xc0, 0x16, 0xda, 0x4c, 0x1c, 0x3d, 0x18, 0x46, 0x63, 0xda,
0x8f, 0x6a,
]);
const DISCARDED_PATH_APDU = Uint8Array.from([
0xe0, 0x1e, 0x00, 0x01, 0x11, 0x10, 0x74, 0x6f, 0x2e, 0x5b, 0x5d, 0x2e, 0x77,
0x61, 0x6c, 0x6c, 0x65, 0x74, 0x73, 0x2e, 0x5b, 0x5d,
]);

describe("SendEIP712FilteringCommand", () => {
describe("getApdu", () => {
Expand All @@ -72,6 +76,19 @@ describe("SendEIP712FilteringCommand", () => {
expect(apdu.getRawApdu()).toStrictEqual(ACTIVATE_APDU);
});

it("Discarded path APDU", () => {
// GIVEN
const args: SendEIP712FilteringCommandArgs = {
type: Eip712FilterType.DiscardedPath,
path: "to.[].wallets.[]",
};
// WHEN
const command = new SendEIP712FilteringCommand(args);
const apdu = command.getApdu();
// THEN
expect(apdu.getRawApdu()).toStrictEqual(DISCARDED_PATH_APDU);
});

it("Message info APDU", () => {
// GIVEN
const args: SendEIP712FilteringCommandArgs = {
Expand All @@ -92,6 +109,7 @@ describe("SendEIP712FilteringCommand", () => {
// GIVEN
const args: SendEIP712FilteringCommandArgs = {
type: Eip712FilterType.Raw,
discarded: false,
displayName: "From",
signature:
"3045022100b820e4dfb1a0cde6dc97d9a34eebb1a4eef0b226262e6788118ab3c7fb79fe3502202d426a388b4c3a8096b3f84412a702ea537770e61ee0727ec1b710c1da520c44",
Expand All @@ -107,6 +125,7 @@ describe("SendEIP712FilteringCommand", () => {
// GIVEN
const args: SendEIP712FilteringCommandArgs = {
type: Eip712FilterType.Token,
discarded: false,
tokenIndex: 1,
signature:
"3045022100ff727847445431e571cd2a0d9db42a7eb62e37877b9bf20e6a96584255347e1902200a6e95b7f8e63b2fab0bef88c747de6a387d06351be5bdc34b2c1f9aea6fdd28",
Expand All @@ -122,6 +141,7 @@ describe("SendEIP712FilteringCommand", () => {
// GIVEN
const args: SendEIP712FilteringCommandArgs = {
type: Eip712FilterType.Amount,
discarded: false,
displayName: "Receive minimum",
tokenIndex: 1,
signature:
Expand All @@ -138,6 +158,7 @@ describe("SendEIP712FilteringCommand", () => {
// GIVEN
const args: SendEIP712FilteringCommandArgs = {
type: Eip712FilterType.Datetime,
discarded: false,
displayName: "Approval expire",
signature:
"3045022100e847166e60f851e3c8d1f44139811898ccd0d3a03aed6c77f8c3993813f479d2022031fe6b6a574b56c5104003cf07900d11ffaf303dc016da4c1c3d184663da8f6a",
Expand All @@ -148,6 +169,24 @@ describe("SendEIP712FilteringCommand", () => {
// THEN
expect(apdu.getRawApdu()).toStrictEqual(DATE_TIME_APDU);
});

it("Discarded filter", () => {
// GIVEN
const args: SendEIP712FilteringCommandArgs = {
type: Eip712FilterType.Raw,
discarded: true,
displayName: "From",
signature:
"3045022100b820e4dfb1a0cde6dc97d9a34eebb1a4eef0b226262e6788118ab3c7fb79fe3502202d426a388b4c3a8096b3f84412a702ea537770e61ee0727ec1b710c1da520c44",
};
// WHEN
const command = new SendEIP712FilteringCommand(args);
const apdu = command.getApdu();
// THEN
expect(apdu.getRawApdu()).toStrictEqual(
Uint8Array.from([...RAW_APDU.slice(0, 2), 0x01, ...RAW_APDU.slice(3)]),
);
});
});

describe("parseResponse", () => {
Expand Down
29 changes: 25 additions & 4 deletions packages/signer/signer-eth/src/internal/app-binder/command/SendEIP712FilteringCommand.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {

export enum Eip712FilterType {
Activation = "activation",
DiscardedPath = "discarded_path",
MessageInfo = "message_info",
Datetime = "datetime",
Raw = "raw",
Expand All @@ -22,24 +23,42 @@ export enum Eip712FilterType {

export type SendEIP712FilteringCommandArgs =
| { type: Eip712FilterType.Activation }
| { type: Eip712FilterType.DiscardedPath; path: string }
| {
type: Eip712FilterType.MessageInfo;
displayName: string;
filtersCount: number;
signature: string;
}
| { type: Eip712FilterType.Datetime; displayName: string; signature: string }
| { type: Eip712FilterType.Token; tokenIndex: number; signature: string }
| { type: Eip712FilterType.Raw; displayName: string; signature: string }
| {
type: Eip712FilterType.Datetime;
discarded: boolean;
displayName: string;
signature: string;
}
| {
type: Eip712FilterType.Token;
discarded: boolean;
tokenIndex: number;
signature: string;
}
| {
type: Eip712FilterType.Raw;
discarded: boolean;
displayName: string;
signature: string;
}
| {
type: Eip712FilterType.Amount;
discarded: boolean;
displayName: string;
tokenIndex: number;
signature: string;
};

const FILTER_TO_P2: Record<Eip712FilterType, number> = {
[Eip712FilterType.Activation]: 0x00,
[Eip712FilterType.DiscardedPath]: 0x01,
[Eip712FilterType.MessageInfo]: 0x0f,
[Eip712FilterType.Datetime]: 0xfc,
[Eip712FilterType.Token]: 0xfd,
Expand All @@ -56,7 +75,7 @@ export class SendEIP712FilteringCommand
const filteringArgs: ApduBuilderArgs = {
cla: 0xe0,
ins: 0x1e,
p1: 0x00,
p1: "discarded" in this.args && this.args.discarded ? 0x01 : 0x00,
p2: FILTER_TO_P2[this.args.type],
};
const builder = new ApduBuilder(filteringArgs);
Expand All @@ -66,6 +85,8 @@ export class SendEIP712FilteringCommand
.encodeInLVFromAscii(this.args.displayName)
.add8BitUIntToData(this.args.filtersCount)
.encodeInLVFromHexa(this.args.signature);
} else if (this.args.type === Eip712FilterType.DiscardedPath) {
builder.encodeInLVFromAscii(this.args.path);
} else if (
this.args.type === Eip712FilterType.Datetime ||
this.args.type === Eip712FilterType.Raw
Expand Down
Loading

0 comments on commit 62638cc

Please sign in to comment.