clawbot
48f1edae57
All checks were successful
check / check (push) Successful in 30s
verifySignedTx compared only from, to, value and data, so a signed transaction could differ from the approval in chain id, nonce, gas limit or any fee field and still be broadcast. Worse, it named the fields it checked and so admitted every field it did not name: a type 4 artifact carrying an EIP-7702 authorization passed verification, paying the approved amount to the approved recipient and, in the same transaction, permanently installing another contract's code at the signer's own account. The check is now an allowlist in both directions. The transaction type must be 0, 1 or 2 — the only types this wallet signs — so no later EIP-2718 type can bring a field along; authorizationList, blobs, blob commitments and blob gas fees are refused by name; and the access list is compared with the approval. Every consequential field is compared and any mismatch refuses outright: the chain id against the selected network (and against the approval when the page fixed one), plus nonce, gas limit, gasPrice, maxFeePerGas and maxPriorityFeePerGas wherever the approval carries a value, together with the fee mechanism the approval implies. Fields the approval does not carry are populated locally by the popup and have no approved value to compare against, so they are held to absolute ceilings instead. Verification then closes by rebuilding the transaction from exactly those checked fields and comparing the unsigned bytes, so an artifact carrying anything this module does not account for is refused without having to be named first. An approved value that is not a number now refuses like every other quantity rather than escaping as a raw BigInt conversion error, which was reported as retryable and left a live button that could never succeed. A failed signing attempt also left a button that could not succeed: the background deleted the approval before it broadcast, so a retry found nothing to sign. The approval is now retired once the request has an outcome, and the background tells the popup which stage failed. A popup that could not sign is retryable; a mismatch spends the approval; a failed broadcast is terminal, because the node may have accepted the transaction and still failed to answer and the popup's retry re-signs at a freshly fetched nonce rather than re-broadcasting the same bytes, which would send the approved transfer twice. Keeping the approval alive for that retry cost it its single use: the handler read it, then verified and broadcast asynchronously, so a second AUTISTMASK_TX_RESPONSE carrying the same id started an independent verify and broadcast instead of finding nothing. With the ordinary dApp approval shape the page fixes no nonce, so two artifacts signed at different nonces both verify and the approved transfer goes out twice; a reloaded approval window during a slow broadcast is enough to send it, since the only guard was popup-local button state. The approval is now claimed synchronously, before the first await, and released only when an attempt fails in a way the user may retry. Same interlock on AUTISTMASK_SIGN_RESPONSE. Verification also compared only the decode against itself: both sides of the closing byte comparison derive from one Transaction.from(), while what is broadcast is the artifact string. An artifact re-encoded with a leading zero byte on an RLP quantity therefore decoded to the approved transaction, passed, and broadcast different bytes. The artifact is now required to be the canonical encoding of its own decode, which is what makes the claim that it *is* the approved transaction true. The background's approval wiring had no tests, which is where both defects lived. It has them now, driven through the real message listener from eth_sendTransaction to broadcast.
15 KiB
15 KiB