Fix: EIP712 signing (#4060)

* fix typed data hashing * fix eip-712 signing * fix tests * bump sdk
5afe · Sep 1, 2022 · f8d7258 · f8d7258
1 parent 180e3ea
commit f8d7258
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 89 deletions.
diff --git a/package.json b/package.json
@@ -76,7 +76,7 @@
   },
   "dependencies": {
     "@ethersproject/hash": "^5.6.1",
-    "@gnosis.pm/safe-apps-sdk": "7.7.0",
+    "@gnosis.pm/safe-apps-sdk": "7.8.0",
     "@gnosis.pm/safe-apps-sdk-v1": "npm:@gnosis.pm/[email protected]",
     "@gnosis.pm/safe-core-sdk": "^2.0.0",
     "@gnosis.pm/safe-deployments": "^1.15.0",

diff --git a/src/routes/safe/components/Apps/components/AppFrame.tsx b/src/routes/safe/components/Apps/components/AppFrame.tsx
@@ -9,6 +9,7 @@ import {
   Methods,
   SignMessageParams,
   RequestId,
+  SignTypedMessageParams,
 } from '@gnosis.pm/safe-apps-sdk'
 import { useSelector } from 'react-redux'
 import { INTERFACE_MESSAGES, Transaction, LowercaseNetworks } from '@gnosis.pm/safe-apps-sdk-v1'
@@ -297,9 +298,9 @@ const AppFrame = ({ appUrl, allowedFeaturesList }: Props): ReactElement => {
     })
 
     communicator?.on(Methods.signTypedMessage, async (msg) => {
-      const { message } = msg.data.params as SignMessageParams
+      const { typedData } = msg.data.params as SignTypedMessageParams
 
-      openSignMessageModal(message, msg.data.id, Methods.signTypedMessage)
+      openSignMessageModal(typedData, msg.data.id, Methods.signTypedMessage)
     })
 
     communicator?.on(Methods.getChainInfo, async () => {

diff --git a/src/routes/safe/components/Apps/components/SignMessageModal/SignMessageModal.test.tsx b/src/routes/safe/components/Apps/components/SignMessageModal/SignMessageModal.test.tsx
@@ -163,7 +163,7 @@ describe('SignMessageModal Component', () => {
       <SignMessageModal
         isOpen
         safeAddress="0x1948fC557ed7219D33138bD2cD52Da7F2047B2bb"
-        message={JSON.stringify(typedMessage)}
+        message={typedMessage}
         safeName="test safe"
         ethBalance="100000000000000000"
         onClose={jest.fn()}
@@ -188,65 +188,15 @@ describe('SignMessageModal Component', () => {
     ).toBeVisible()
   })
 
-  test('If message is invalid JSON data, modal should be closed', () => {
-    const onCloseFn = jest.fn()
-    const typedMessageStr = `{
-        Person: [
-          { name: 'name', type: 'string' },
-          { name: 'wallet', type: 'address' },
-        ],
-        Mail: [
-          { name: 'from', type: 'Person' },
-          { name: 'to', type: 'Person' },
-          { name: 'contents', type: 'string' },
-        ],
-      },
-      primaryType: 'Mail',
-      domain: {
-        name: 'Ether Mail',
-        version: '1',
-        chainId: 1,
-        verifyingContract: '0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC',
-      },
-      message: {
-        from: {
-          name: 'Cow',
-          wallet: '0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826',
-        },
-        to: {
-          name: 'Bob',
-          wallet: '0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB',
-        },
-        contents: 'Hello, Bob!',
-      },
-    }`
-
-    render(
-      <SignMessageModal
-        isOpen
-        safeAddress="0x1948fC557ed7219D33138bD2cD52Da7F2047B2bb"
-        message={typedMessageStr}
-        safeName="test safe"
-        ethBalance="100000000000000000"
-        onClose={onCloseFn}
-        onUserConfirm={jest.fn()}
-        onTxReject={jest.fn()}
-        requestId="1"
-        method={Methods.signTypedMessage}
-        app={getEmptySafeApp()}
-      />,
-    )
-    expect(onCloseFn).toHaveBeenCalled()
-  })
-
   test('If message is invalid typed data, modal should be closed', () => {
     const onCloseFn = jest.fn()
-    const typedMessageStr = `{"test": "test"}`
+    const typedMessageStr = { test: 'test' }
 
     render(
       <SignMessageModal
         isOpen
         safeAddress="0x1948fC557ed7219D33138bD2cD52Da7F2047B2bb"
+        // @ts-expect-error - invalid typed message
         message={typedMessageStr}
         safeName="test safe"
         ethBalance="100000000000000000"

diff --git a/src/routes/safe/components/Apps/components/SignMessageModal/index.tsx b/src/routes/safe/components/Apps/components/SignMessageModal/index.tsx
@@ -1,4 +1,4 @@
-import { RequestId, Methods } from '@gnosis.pm/safe-apps-sdk'
+import { RequestId, Methods, EIP712TypedData, isObjectEIP712TypedData } from '@gnosis.pm/safe-apps-sdk'
 import { ReactElement } from 'react'
 import { useSelector } from 'react-redux'
 import { hexToUtf8, isHexStrict } from 'web3-utils'
@@ -16,7 +16,7 @@ import { _TypedDataEncoder } from '@ethersproject/hash'
 export type SignMessageModalProps = {
   isOpen: boolean
   app: SafeApp
-  message: string
+  message: string | EIP712TypedData
   safeAddress: string
   safeName: string
   requestId: RequestId
@@ -42,40 +42,32 @@ const convertToHumanReadableMessage = (message: string): string => {
   return humanReadableMessage
 }
 
-export const SignMessageModal = ({ message, isOpen, method, ...rest }: SignMessageModalProps): ReactElement => {
+export const SignMessageModal = ({ message, isOpen, method, ...rest }: SignMessageModalProps): ReactElement | null => {
   const web3 = getWeb3ReadOnly()
   const networkId = useSelector(currentChainId)
   const txRecipient = getSignMessageLibAddress(networkId) || ZERO_ADDRESS
   let txData, readableData
-  if (method == Methods.signMessage) {
+  if (method == Methods.signMessage && typeof message === 'string') {
     txData = getSignMessageLibContractInstance(web3, networkId)
       .methods.signMessage(web3.eth.accounts.hashMessage(message))
       .encodeABI()
     readableData = convertToHumanReadableMessage(message)
   } else if (method == Methods.signTypedMessage) {
-    // check if the message is a valid typed data
     try {
-      const typedData = JSON.parse(message)
-      if (!('domain' in typedData && 'types' in typedData && 'message' in typedData)) {
+      if (!isObjectEIP712TypedData(message)) {
         throw new Error('Invalid typed data')
       }
-      readableData = JSON.stringify(typedData, undefined, 4)
+      readableData = JSON.stringify(message, undefined, 4)
     } catch (e) {
       // As the signing method is SignTypedMessage, the message should be a valid JSON.
       // When it is not, we will reject the tx and close the modal.
       rest.onTxReject(rest.requestId)
       rest.onClose()
+      return null
     }
-    // Here we firstly convert the typed message to safe typed message which contains the safe address and the current chainId,
-    // then we hash it using ethers.utils._TypedDataEncoder.
+
     txData = getSignMessageLibContractInstance(web3, networkId)
-      .methods.signMessage(
-        _TypedDataEncoder.hash(
-          { verifyingContract: rest.safeAddress, chainId: parseInt(networkId) },
-          { SafeMessage: [{ type: 'string', name: 'message' }] },
-          { message: message },
-        ),
-      )
+      .methods.signMessage(_TypedDataEncoder.hash(message.domain, message.types, message.message))
       .encodeABI()
   } else {
     // Unsupported method

diff --git a/src/routes/safe/components/Apps/hooks/useSignMessageModal.ts b/src/routes/safe/components/Apps/hooks/useSignMessageModal.ts
@@ -1,7 +1,12 @@
 import { useState, useCallback } from 'react'
-import { Methods } from '@gnosis.pm/safe-apps-sdk'
+import { EIP712TypedData, Methods } from '@gnosis.pm/safe-apps-sdk'
 
-type StateType = { isOpen: boolean; message: string; requestId: string; method: Methods }
+type StateType = {
+  isOpen: boolean
+  requestId: string
+  message: string | EIP712TypedData
+  method: Methods.signMessage | Methods.signTypedMessage
+}
 
 const INITIAL_MODAL_STATE: StateType = {
   isOpen: false,
@@ -10,20 +15,27 @@ const INITIAL_MODAL_STATE: StateType = {
   method: Methods.signMessage,
 }
 
-type ReturnType = [StateType, (message: string, requestId: string, method: Methods) => void, () => void]
+type ReturnType = [
+  StateType,
+  (message: string | EIP712TypedData, requestId: string, method: Methods) => void,
+  () => void,
+]
 
 export const useSignMessageModal = (): ReturnType => {
   const [signMessageModalState, setSignMessageModalState] = useState<StateType>(INITIAL_MODAL_STATE)
 
-  const openSignMessageModal = useCallback((message: string, requestId: string, method: Methods) => {
-    setSignMessageModalState({
-      ...INITIAL_MODAL_STATE,
-      isOpen: true,
-      message,
-      requestId,
-      method,
-    })
-  }, [])
+  const openSignMessageModal = useCallback(
+    (message: string | EIP712TypedData, requestId: string, method: Methods.signTypedMessage | Methods.signMessage) => {
+      setSignMessageModalState({
+        ...INITIAL_MODAL_STATE,
+        isOpen: true,
+        message,
+        requestId,
+        method,
+      })
+    },
+    [],
+  )
 
   const closeSignMessageModal = useCallback(() => {
     setSignMessageModalState(INITIAL_MODAL_STATE)

diff --git a/yarn.lock b/yarn.lock
@@ -2155,10 +2155,10 @@
     "@gnosis.pm/safe-react-gateway-sdk" "^2.5.6"
     ethers "^5.4.7"
 
-"@gnosis.pm/safe-apps-sdk@7.7.0":
-  version "7.7.0"
-  resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-apps-sdk/-/safe-apps-sdk-7.7.0.tgz#dc3621634bd42f1d24cbe4342fd85592cd1491cd"
-  integrity sha512-MuiH09q/5DFfHQLrwNW7JSvGEMGqrS3jNJy9UKZHW77rxzmZssHX1l3+p5whTbtrcvLbxQgcp5egMrdw/sj0qw==
+"@gnosis.pm/safe-apps-sdk@7.8.0":
+  version "7.8.0"
+  resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-apps-sdk/-/safe-apps-sdk-7.8.0.tgz#295ab9d563b94208e3042495cdba787fa035c71e"
+  integrity sha512-kO8fJi1ebiKN9qH1NdDToVBuDQQ0U9NkL467U+84LtNTx5PzUJIu6O7tb4nZD24e/OItinf5W8GDWhhfZeiqOA==
   dependencies:
     "@gnosis.pm/safe-react-gateway-sdk" "^3.1.3"
     ethers "^5.6.8"