Compare commits

..
1 Commits
Author SHA1 Message Date
sneak eaaf1593c0 harden: warn for token-permission typed data and show the primary type ethers signs (closes #400)
check / check (push) Successful in 49s
e2e / e2e-chrome (push) Successful in 1m44s
e2e / e2e-firefox (push) Successful in 31s
The typed-data screen listed a Permit or Permit2 signature as plain
key/value lines, like a sign-in message. For typed data signed as Permit
or as one of Permit2's types it now shows a red warning naming the
spender and each token and amount, read only from the fields the signed
type declares; whatever they do not give reads Unknown.

The screen printed the page's primaryType, but ethers signs the type it
derives from types. It now shows the derived type, and typed data whose
stated type is missing or differs is refused: error line, Sign disabled,
and checked again where signing starts.

Deviation: the warning names no deadline or expiry; see the issue.
Judgement call: DAI's older permit and Permit2's batch and witness
transfer types are recognised too.

Model: opus-5-5
2026-10-03 14:38:06 +00:00
4 changed files with 13 additions and 109 deletions
+1 -2
View File
@@ -1812,8 +1812,7 @@ view would leave a wallet one click from deletion.
spender's full address and, for each token, its symbol, full address and
amount (`Unlimited` for the largest amount the field holds). These are
read only from the fields the signed type declares, never from other keys
the site puts in the message, except a `Permit`'s token, which is the
domain's `verifyingContract`; any those fields do not give is shown as
the site puts in the message; any those fields do not give is shown as
`Unknown`, and the domain, type and message lines still follow. Only typed
data that cannot be read at all is shown as raw text.
- Password input and an error line
+9 -10
View File
@@ -54,16 +54,15 @@ but the review is broader than any of them.
that name) or as one of Permit2's six signature types,
`src/popup/views/approval.js` now shows a red warning at the top of the
message naming the spender and each token and amount, read only from the
fields the signed type declares (a `Permit`'s token is the domain's
`verifyingContract`), with `Unlimited` for the largest amount the field holds,
the existing unknown-scale wording otherwise, and `Unknown` for whatever those
fields do not give. The screen printed the page's `primaryType`, but ethers
signs the type it derives from `types`; the screen now shows the derived type,
and typed data whose stated type is missing or differs, or that cannot be
read, is shown with an error line and Sign disabled, and is refused again
where signing starts. The warning names no deadline or expiry: those fields
mean different things across the shapes, and a date could read as the
permission ending when it does not.
fields the signed type declares, with `Unlimited` for the largest amount the
field holds, the existing unknown-scale wording otherwise, and `Unknown` for
whatever those fields do not give. The screen printed the page's
`primaryType`, but ethers signs the type it derives from `types`; the screen
now shows the derived type, and typed data whose stated type is missing or
differs, or that cannot be read, is shown with an error line and Sign
disabled, and is refused again where signing starts. The warning names no
deadline or expiry: those fields mean different things across the shapes, and
a date could read as the permission ending when it does not.
- 2026-09-21: The network fee a transaction can commit is bounded by the product
of the gas limit and the fee per gas, not by each field alone, and the
wallet's own send is bounded the same way
+1 -2
View File
@@ -492,8 +492,7 @@ function amountOrNull(value) {
// type ethers signs, `Permit` or one of Permit2's six signature types: a
// contract checks the type's exact name, so a renamed copy of one of these
// would not be honoured. Everything named is read only from the fields that
// type declares, except a `Permit`'s token, which is the domain's
// `verifyingContract`; whatever they do not give is shown as `Unknown`.
// type declares, and whatever they do not give is shown as `Unknown`.
function permitWarningHtml(types, primaryType, domain, message) {
// Each entry is a token, the amount, and the largest value its amount
// field holds, which the contract treats as unlimited.
+2 -95
View File
@@ -9,9 +9,8 @@
//
// ethers signs only the fields a type declares in `types` and drops any other
// key in the message, so the warning reads the spender, tokens and amounts
// from those fields alone, except a `Permit`'s token, which is the domain's
// `verifyingContract`: a key the page adds beside them must not change what
// the warning says.
// from those fields alone: a key the page adds beside them must not change
// what the warning says.
//
// ethers signs the type it derives from `types`, not the page's
// `primaryType`, so the screen names the derived type, and a request whose
@@ -78,75 +77,6 @@ const PERMIT_SINGLE = {
},
};
// Permit2's PermitBatch: 5 USDC and 7 DAI, a line for each token.
const PERMIT_BATCH = {
types: {
EIP712Domain: PERMIT_SINGLE.types.EIP712Domain,
PermitBatch: [
{ name: "details", type: "PermitDetails[]" },
{ name: "spender", type: "address" },
{ name: "sigDeadline", type: "uint256" },
],
PermitDetails: PERMIT_SINGLE.types.PermitDetails,
},
primaryType: "PermitBatch",
domain: PERMIT_SINGLE.domain,
message: {
details: [
{
token: USDC,
amount: "5000000",
expiration: "1790000000",
nonce: "0",
},
{
token: DAI,
amount: "7000000000000000000",
expiration: "1790000000",
nonce: "0",
},
],
spender: SPENDER,
sigDeadline: "1790000000",
},
};
// One of Permit2's four transfer types, letting SPENDER take 5 USDC. The
// batch types take a list of `TokenPermissions`; the witness types add a
// struct of the site's own as their last field.
function permit2Transfer(primaryType, { batch = false, witness = false }) {
const fields = [
{
name: "permitted",
type: batch ? "TokenPermissions[]" : "TokenPermissions",
},
{ name: "spender", type: "address" },
{ name: "nonce", type: "uint256" },
{ name: "deadline", type: "uint256" },
];
const types = {
EIP712Domain: PERMIT_SINGLE.types.EIP712Domain,
[primaryType]: fields,
TokenPermissions: [
{ name: "token", type: "address" },
{ name: "amount", type: "uint256" },
],
};
const permitted = { token: USDC, amount: "5000000" };
const message = {
permitted: batch ? [permitted] : permitted,
spender: SPENDER,
nonce: "0",
deadline: "1790000000",
};
if (witness) {
fields.push({ name: "witness", type: "Order" });
types.Order = [{ name: "recipient", type: "address" }];
message.witness = { recipient: OWNER };
}
return { types, primaryType, domain: PERMIT_SINGLE.domain, message };
}
// EIP-2612's Permit for 5 USDC; the token is the domain's contract.
const PERMIT = {
types: {
@@ -251,29 +181,6 @@ describe("a token permission is warned about, naming spender and amount", () =>
expect(warning).toContain("Unlimited");
});
test("a Permit2 PermitBatch names both tokens and both amounts", () => {
const warning = warningOf(PERMIT_BATCH);
expect(warning).toContain(WARNING);
expect(warning).toContain(SPENDER);
expect(warning).toContain(USDC);
expect(warning).toContain("5.0000 USDC");
expect(warning).toContain(DAI);
expect(warning).toContain("7.0000 DAI");
});
test.each([
["PermitTransferFrom", {}],
["PermitWitnessTransferFrom", { witness: true }],
["PermitBatchTransferFrom", { batch: true }],
["PermitBatchWitnessTransferFrom", { batch: true, witness: true }],
])("a Permit2 %s", (primaryType, shape) => {
const warning = warningOf(permit2Transfer(primaryType, shape));
expect(warning).toContain(WARNING);
expect(warning).toContain(SPENDER);
expect(warning).toContain(USDC);
expect(warning).toContain("5.0000 USDC");
});
test("an EIP-2612 Permit for 5 USDC", () => {
const warning = warningOf(PERMIT);
expect(warning).toContain(WARNING);