Skip to content

Commit

Permalink
fix: eip 712 validation (#894)
Browse files Browse the repository at this point in the history
* chore: remove inaccurate comment

* feat: invalid signTypedData buttons in playground

* feat: display invalid addresses in orange

* feat: detect invalid verifying contract address

* fix: style consistency

* fix: margin around gas footer
  • Loading branch information
0xKheops authored Jul 3, 2023
1 parent 9bf810d commit 0bfd7da
Show file tree
Hide file tree
Showing 9 changed files with 184 additions and 95 deletions.
1 change: 0 additions & 1 deletion apps/extension/src/core/domains/signing/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export default class SigningHandler extends ExtensionHandler {
hostName: ok ? hostName : undefined,
}

// an empty registry is sufficient, we don't need metadata here
let registry = new TypeRegistry()

if (isJsonPayload(payload)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { AccountJsonHardwareEthereum } from "@core/domains/accounts/types"
import { AppPill } from "@talisman/components/AppPill"
import Grid from "@talisman/components/Grid"
import { SimpleButton } from "@talisman/components/SimpleButton"
import { Content, Footer, Header } from "@ui/apps/popup/Layout"
import { EthSignBodyMessage } from "@ui/domains/Sign/Ethereum/EthSignBodyMessage"
import { useEthSignMessageRequest } from "@ui/domains/Sign/SignRequestContext"
import { Suspense, lazy, useEffect, useMemo } from "react"
import { useTranslation } from "react-i18next"
import { Button } from "talisman-ui"

import { SignContainer } from "./common"
import { SignAccountAvatar } from "./SignAccountAvatar"
Expand All @@ -15,8 +14,18 @@ const LedgerEthereum = lazy(() => import("@ui/domains/Sign/LedgerEthereum"))

export const EthSignMessageRequest = () => {
const { t } = useTranslation("request")
const { url, request, approve, approveHardware, reject, status, message, account, network } =
useEthSignMessageRequest()
const {
url,
request,
approve,
approveHardware,
reject,
status,
message,
account,
network,
isValid,
} = useEthSignMessageRequest()

const { processing, errorMessage } = useMemo(() => {
return {
Expand Down Expand Up @@ -52,19 +61,19 @@ export const EthSignMessageRequest = () => {
onReject={reject}
/>
) : (
<Grid>
<SimpleButton disabled={processing} onClick={reject}>
<div className="grid w-full grid-cols-2 gap-12">
<Button disabled={processing} onClick={reject}>
{t("Cancel")}
</SimpleButton>
<SimpleButton
disabled={processing}
</Button>
<Button
disabled={processing || !isValid}
processing={processing}
primary
onClick={approve}
>
{t("Approve")}
</SimpleButton>
</Grid>
</Button>
</div>
)}
</>
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { AccountJsonHardwareEthereum } from "@core/domains/accounts/types"
import { EthPriorityOptionName } from "@core/domains/signing/types"
import { AppPill } from "@talisman/components/AppPill"
import Grid from "@talisman/components/Grid"
import { SimpleButton } from "@talisman/components/SimpleButton"
import { WithTooltip } from "@talisman/components/Tooltip"
import { InfoIcon } from "@talisman/theme/icons"
import { useQuery } from "@tanstack/react-query"
Expand Down Expand Up @@ -62,10 +60,6 @@ const SignContainer = styled(Container)`
white-space: nowrap;
}
${SimpleButton} {
width: auto;
}
.center {
text-align: center;
}
Expand All @@ -86,10 +80,6 @@ const SignContainer = styled(Container)`
}
}
${Grid} {
margin-top: 1.6rem;
}
.error {
color: var(--color-status-error);
max-width: 100%;
Expand Down Expand Up @@ -246,7 +236,7 @@ export const EthSignTransactionRequest = () => {
</div>
<Suspense fallback={null}>
{transaction && txDetails && network?.nativeToken ? (
<div className="gasInfo mt-8">
<div className="gasInfo my-8">
<div>
<div>
{t("Estimated Fee")}{" "}
Expand Down Expand Up @@ -292,7 +282,6 @@ export const EthSignTransactionRequest = () => {
transaction ? (
<LedgerEthereum
manualSend
className="mt-6"
method="transaction"
payload={transaction}
account={account as AccountJsonHardwareEthereum}
Expand All @@ -306,19 +295,19 @@ export const EthSignTransactionRequest = () => {
</Button>
)
) : (
<Grid>
<SimpleButton disabled={processing} onClick={reject}>
<div className="grid w-full grid-cols-2 gap-12">
<Button disabled={processing} onClick={reject}>
{t("Cancel")}
</SimpleButton>
<SimpleButton
</Button>
<Button
disabled={!transaction || processing || isLoading || !isValid}
processing={processing}
primary
onClick={approve}
>
{t("Approve")}
</SimpleButton>
</Grid>
</Button>
</div>
)}
</Suspense>
</Footer>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import { AccountJsonHardwareSubstrate, AccountJsonQr } from "@core/domains/accounts/types"
import { SignerPayloadRaw } from "@core/domains/signing/types"
import { AppPill } from "@talisman/components/AppPill"
import { Box } from "@talisman/components/Box"
import { SimpleButton } from "@talisman/components/SimpleButton"
import { Content, Footer, Header } from "@ui/apps/popup/Layout"
import { AccountPill } from "@ui/domains/Account/AccountPill"
import { Message } from "@ui/domains/Sign/Message"
import { QrSubstrate } from "@ui/domains/Sign/Qr/QrSubstrate"
import { usePolkadotSigningRequest } from "@ui/domains/Sign/SignRequestContext"
import { FC, Suspense, lazy, useEffect, useMemo } from "react"
import { useTranslation } from "react-i18next"
import { Button } from "talisman-ui"

import { Container } from "./common"
import { SignAccountAvatar } from "./SignAccountAvatar"
Expand Down Expand Up @@ -69,19 +68,14 @@ export const PolkadotSignMessageRequest: FC = () => {
{account && request && (
<>
{account.origin !== "HARDWARE" && account.origin !== "QR" && (
<Box flex fullwidth gap={2.4}>
<SimpleButton disabled={processing} onClick={reject}>
<div className="grid w-full grid-cols-2 gap-12">
<Button disabled={processing} onClick={reject}>
{t("Cancel")}
</SimpleButton>
<SimpleButton
disabled={processing}
processing={processing}
primary
onClick={approve}
>
</Button>
<Button disabled={processing} processing={processing} primary onClick={approve}>
{t("Approve")}
</SimpleButton>
</Box>
</Button>
</div>
)}
{account.origin === "HARDWARE" && (
<Suspense fallback={null}>
Expand Down
32 changes: 27 additions & 5 deletions apps/extension/src/ui/domains/Sign/Ethereum/EthSignBodyMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,25 @@ import { EthSignRequest } from "@core/domains/signing/types"
import { log } from "@core/log"
import { isHexString, stripHexPrefix } from "@ethereumjs/util"
import * as Sentry from "@sentry/browser"
import { classNames } from "@talismn/util"
import { classNames, isEthereumAddress } from "@talismn/util"
import { Message } from "@ui/domains/Sign/Message"
import { useEvmNetwork } from "@ui/hooks/useEvmNetwork"
import { dump as convertToYaml } from "js-yaml"
import { FC, useMemo } from "react"
import { useTranslation } from "react-i18next"

import { SignAlertMessage } from "../SignAlertMessage"
import { SignParamAccountButton, SignParamNetworkAddressButton } from "./shared"

const useEthSignMessage = (request: EthSignRequest) => {
const { isTypedData, typedMessage, verifyingAddress, chainId, ethChainId } = useMemo(() => {
const {
isTypedData,
typedMessage,
verifyingAddress,
chainId,
ethChainId,
isInvalidVerifyingContract,
} = useMemo(() => {
try {
const isTypedData = Boolean(request?.method?.startsWith("eth_signTypedData"))
const typedMessage = isTypedData ? JSON.parse(request.request) : undefined
Expand All @@ -22,7 +30,15 @@ const useEthSignMessage = (request: EthSignRequest) => {
? parseInt(typedMessage.domain?.chainId)
: undefined
const ethChainId = request.ethChainId
return { isTypedData, typedMessage, verifyingAddress, chainId, ethChainId }
const isInvalidVerifyingContract = verifyingAddress && !isEthereumAddress(verifyingAddress)
return {
isTypedData,
typedMessage,
verifyingAddress,
chainId,
ethChainId,
isInvalidVerifyingContract,
}
} catch (err) {
log.error(err)
return { isTypedData: false }
Expand Down Expand Up @@ -52,7 +68,7 @@ const useEthSignMessage = (request: EthSignRequest) => {
return request.request
}, [request.request, typedMessage])

return { isTypedData, text, verifyingAddress, chainId, ethChainId }
return { isTypedData, text, verifyingAddress, chainId, ethChainId, isInvalidVerifyingContract }
}

export type EthSignBodyMessageProps = {
Expand All @@ -62,7 +78,8 @@ export type EthSignBodyMessageProps = {

export const EthSignBodyMessage: FC<EthSignBodyMessageProps> = ({ account, request }) => {
const { t } = useTranslation("request")
const { isTypedData, text, verifyingAddress, ethChainId } = useEthSignMessage(request)
const { isTypedData, text, verifyingAddress, ethChainId, isInvalidVerifyingContract } =
useEthSignMessage(request)
const evmNetwork = useEvmNetwork(ethChainId)

return (
Expand All @@ -87,6 +104,11 @@ export const EthSignBodyMessage: FC<EthSignBodyMessageProps> = ({ account, reque
className={classNames("w-full grow", isTypedData && "whitespace-pre text-xs")}
text={text}
/>
{isInvalidVerifyingContract && (
<SignAlertMessage type="error" className="mt-8">
{t("Verifying contract's address is invalid.")}
</SignAlertMessage>
)}
</div>
)
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { WithTooltip } from "@talisman/components/Tooltip"
import { isEthereumAddress } from "@talismn/util"
import { AccountIcon } from "@ui/domains/Account/AccountIcon"
import { Address } from "@ui/domains/Account/Address"
import AccountAvatar from "@ui/domains/Account/Avatar"
import { useAccountByAddress } from "@ui/hooks/useAccountByAddress"
import { FC } from "react"
import { FC, useMemo } from "react"
import { Tooltip, TooltipContent, TooltipTrigger } from "talisman-ui"

import { SignParamButton, SignParamButtonProps } from "./SignParamButton"

Expand All @@ -16,6 +17,7 @@ export const SignParamAccountButton: FC<SignParamAccountButtonProps> = ({
withIcon,
}) => {
const account = useAccountByAddress(address)
const isInvalidAddress = useMemo(() => !isEthereumAddress(address), [address])

return (
<SignParamButton
Expand All @@ -24,26 +26,27 @@ export const SignParamAccountButton: FC<SignParamAccountButtonProps> = ({
withIcon={withIcon}
iconPrefix={
account ? (
<AccountAvatar
<AccountIcon
className="!h-[1.65rem] !text-[1.65rem] !leading-none"
address={account.address}
/>
) : (
<AccountAvatar
<AccountIcon
type="polkadot-identicon"
className="!h-[1.65rem] !text-[1.65rem] !leading-none"
address={address}
/>
)
}
contentClassName={isInvalidAddress ? "!text-alert-warn" : undefined}
>
{account?.name ? (
<WithTooltip
tooltip={address}
className="inline-block h-[1.2em] max-w-[16rem] overflow-hidden overflow-ellipsis whitespace-nowrap align-baseline"
>
{account.name}
</WithTooltip>
<Tooltip>
<TooltipTrigger asChild>
<span>{account.name}</span>
</TooltipTrigger>
<TooltipContent>{address}</TooltipContent>
</Tooltip>
) : (
<Address startCharCount={6} endCharCount={4} address={address} />
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { CustomEvmNetwork, EvmNetwork } from "@core/domains/ethereum/types"
import { WithTooltip } from "@talisman/components/Tooltip"
import { isEthereumAddress } from "@talismn/util"
import { Address } from "@ui/domains/Account/Address"
import { AssetLogo } from "@ui/domains/Asset/AssetLogo"
import useToken from "@ui/hooks/useToken"
import { FC } from "react"
import { FC, useMemo } from "react"
import { Tooltip, TooltipContent, TooltipTrigger } from "talisman-ui"

import { SignParamButton } from "./SignParamButton"

Expand All @@ -21,6 +22,7 @@ export const SignParamNetworkAddressButton: FC<SignParamNetworkAddressButtonProp
className,
}) => {
const nativeToken = useToken(network.nativeToken?.id)
const isInvalidAddress = useMemo(() => !isEthereumAddress(address), [address])

return (
<SignParamButton
Expand All @@ -34,14 +36,15 @@ export const SignParamNetworkAddressButton: FC<SignParamNetworkAddressButtonProp
}
withIcon
className={className}
contentClassName={isInvalidAddress ? "text-alert-warn" : undefined}
>
{name ? (
<WithTooltip
tooltip={address}
className="inline-block max-w-[16rem] overflow-hidden text-ellipsis whitespace-nowrap"
>
{name}
</WithTooltip>
<Tooltip>
<TooltipTrigger asChild>
<span>{name}</span>
</TooltipTrigger>
<TooltipContent>{address}</TooltipContent>
</Tooltip>
) : (
<Address startCharCount={6} endCharCount={4} address={address} />
)}
Expand Down
Loading

0 comments on commit 0bfd7da

Please sign in to comment.