Skip to content

Commit

Permalink
Tweaks and creaks: Disable UNS, fix Ledger signing on built-in non-ET…
Browse files Browse the repository at this point in the history
…H/MATIC networks (#3746)

These are unrelated changes: UNS is disabled due to compromise during
the late-last-week SquareSpace domain theft issues, while the Ledger
signing issue is long-standing. The Ledger signing issue is partial—it
doesn't work for custom networks, and it is handled in a way that's
different for EIP191 messages than for other messages; further
refinement is needed to fix it the rest of the way.

Latest build:
[extension-builds-3746](https://github.com/tahowallet/extension/suites/25994003749/artifacts/1700133481)
(as of Mon, 15 Jul 2024 02:27:54 GMT).
  • Loading branch information
Shadowfiend authored Jul 15, 2024
2 parents 0024916 + 7a25914 commit fa852d4
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 20 deletions.
2 changes: 0 additions & 2 deletions background/constants/networks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,6 @@ export const TEST_NETWORK_BY_CHAIN_ID = new Set(
[SEPOLIA, ARBITRUM_SEPOLIA].map((network) => network.chainID),
)

export const NETWORK_FOR_LEDGER_SIGNING = [ETHEREUM, POLYGON]

// Networks that are not added to this struct will
// not have an in-wallet Swap page
export const CHAIN_ID_TO_0X_API_BASE: {
Expand Down
13 changes: 8 additions & 5 deletions background/services/ledger/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
import {
isEIP1559TransactionRequest,
isKnownTxType,
sameNetwork,
SignedTransaction,
TransactionRequestWithNonce,
} from "../../networks"
Expand All @@ -25,7 +24,7 @@ import { ServiceCreatorFunction, ServiceLifecycleEvents } from "../types"
import logger from "../../lib/logger"
import { getOrCreateDB, LedgerAccount, LedgerDatabase } from "./db"
import { ethersTransactionFromTransactionRequest } from "../chain/utils"
import { NETWORK_FOR_LEDGER_SIGNING } from "../../constants"
import { ETHEREUM } from "../../constants"
import { normalizeEVMAddress } from "../../lib/utils"
import { AddressOnNetwork } from "../../accounts"

Expand Down Expand Up @@ -542,10 +541,14 @@ export default class LedgerService extends BaseService<Events> {
{ address, network }: AddressOnNetwork,
hexDataToSign: HexString,
): Promise<string> {
// Currently the service assumes the Eth app, which requires a network that
// uses the same derivation path as Ethereum, or one that starts with the
// same components.
// FIXME This should take a `LedgerAccountSigner` and use `checkCanSign`
// FIXME like other signing methods.
if (
!NETWORK_FOR_LEDGER_SIGNING.find((supportedNetwork) =>
sameNetwork(network, supportedNetwork),
)
network.derivationPath !== ETHEREUM.derivationPath &&
!network.derivationPath?.startsWith(ETHEREUM.derivationPath ?? "")
) {
throw new Error("Unsupported network for Ledger signing")
}
Expand Down
25 changes: 17 additions & 8 deletions background/services/name/resolvers/uns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ const UNS_SUPPORTED_NETWORKS = [ETHEREUM, POLYGON]
/**
* Lookup a UNS domain name and fetch the owners address
*/
// FIXME UNS issues
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const lookupUNSDomain = async (domain: string) => {
const response = await fetchJson({
url: `https://resolve.unstoppabledomains.com/domains/${domain}`,
Expand Down Expand Up @@ -149,10 +151,13 @@ export default function unsResolver(): NameResolver<"UNS"> {
},

async lookUpAddressForName({
name,
network,
name: _,
network: __,
}: NameOnNetwork): Promise<AddressOnNetwork | undefined> {
// We try to resolve the name using unstoppable domains resolution
// FIXME Restore body once UNS is back in action and we have a new API key.
return undefined

/* We try to resolve the name using unstoppable domains resolution
const address = (await lookupUNSDomain(name))?.meta?.owner
if (address === undefined || address === null) {
Expand All @@ -162,11 +167,13 @@ export default function unsResolver(): NameResolver<"UNS"> {
return {
address,
network,
}
} */
},
async lookUpAvatar(
addressOrNameOnNetwork: AddressOnNetwork | NameOnNetwork,
) {
async lookUpAvatar(_: AddressOnNetwork | NameOnNetwork) {
// FIXME Restore body once UNS is back in action and we have a new API key.
return undefined

/*
const { network } = addressOrNameOnNetwork
const { address } =
"address" in addressOrNameOnNetwork
Expand Down Expand Up @@ -200,12 +207,14 @@ export default function unsResolver(): NameResolver<"UNS"> {
return {
uri: avatarUrn,
network,
}
} */
},
async lookUpNameForAddress({
address,
network,
}: AddressOnNetwork): Promise<NameOnNetwork | undefined> {
return Promise.resolve(undefined)

// Get all the records associated with the particular ETH address
const data = (await reverseLookupAddress(address))?.data
// Since for a given address you can have multiple UNS records, we just pick the first one
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,21 @@ function SignerLedgerSigningTypedData({
typedData: EIP712TypedData
}): ReactElement {
const { EIP712Domain: _, ...typesForSigning } = typedData.types
const domainHash = _TypedDataEncoder

// Below, we prefix the 0x so that we can uppercase the hex characters
// without uppercasing the X. This is because the Ledger displays hex
// characters all uppercase for this operation, but an uppercased X
// both makes our display and the Ledger's less accurate and makes it
// harder to scan the values.
const domainHash = `0x${_TypedDataEncoder
.hashDomain(typedData.domain)
.toUpperCase()
const messageHash = _TypedDataEncoder
.substring(2)
.toUpperCase()}`
const messageHash = `0x${_TypedDataEncoder
.from(typesForSigning)
.hash(typedData.message)
.toUpperCase()
.substring(2)
.toUpperCase()}`

return (
<TransactionDetailContainer>
Expand Down
7 changes: 6 additions & 1 deletion ui/pages/Send.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,12 @@ export default function Send(): ReactElement {
</button>
)}
{addressErrorMessage !== undefined && (
<p className="error">{addressErrorMessage}</p>
<p
className="error"
title="Note: UNS temporarily disabled for security reasons."
>
⚠️ {addressErrorMessage}
</p>
)}
</div>
<div className="send_footer standard_width_padded">
Expand Down

0 comments on commit fa852d4

Please sign in to comment.