From 3049c6016b40e676dba7d6ae00c5af8ddad8a50e Mon Sep 17 00:00:00 2001 From: Stefan Date: Tue, 2 Jul 2024 00:02:05 +0300 Subject: [PATCH] feat(dapps) extend and improve sign Extend support for legacy `eth_sign` and `eth_signTypedData` methods. Keep the `eth_sign` using the legacy method for compatibility Use the newly added status-go apis for a safer implementation of signing typed data by providing chain validation. Closes: #15361 --- .../wallet_connect/controller.nim | 7 ++- .../service/wallet_connect/service.nim | 7 ++- src/backend/wallet_connect.nim | 47 ++++++++++++++----- storybook/pages/DAppsWorkflowPage.qml | 10 +++- .../qmlTests/tests/tst_DAppsWorkflow.qml | 6 +-- .../services/dapps/DAppsRequestHandler.qml | 45 +++++++++++++----- .../services/dapps/types/SessionRequest.qml | 17 ++++++- .../popups/walletconnect/DAppRequestModal.qml | 11 +++++ ui/imports/shared/stores/DAppsStore.qml | 9 +++- vendor/status-go | 2 +- 10 files changed, 123 insertions(+), 38 deletions(-) diff --git a/src/app/modules/shared_modules/wallet_connect/controller.nim b/src/app/modules/shared_modules/wallet_connect/controller.nim index 302a05f263..578b3d6121 100644 --- a/src/app/modules/shared_modules/wallet_connect/controller.nim +++ b/src/app/modules/shared_modules/wallet_connect/controller.nim @@ -56,11 +56,14 @@ QtObject: self.userAuthenticationResult(topic, id, success, password, pin) ) + proc signMessageUnsafe*(self: Controller, address: string, password: string, message: string): string {.slot.} = + return self.service.signMessageUnsafe(address, password, message) + proc signMessage*(self: Controller, address: string, password: string, message: string): string {.slot.} = return self.service.signMessage(address, password, message) - proc signTypedDataV4*(self: Controller, address: string, password: string, typedDataJson: string): string {.slot.} = - return self.service.signTypedDataV4(address, password, typedDataJson) + proc safeSignTypedData*(self: Controller, address: string, password: string, typedDataJson: string, chainId: int, legacy: bool): string {.slot.} = + return self.service.safeSignTypedData(address, password, typedDataJson, chainId, legacy) proc signTransaction*(self: Controller, address: string, chainId: int, password: string, txJson: string): string {.slot.} = return self.service.signTransaction(address, chainId, password, txJson) diff --git a/src/app_service/service/wallet_connect/service.nim b/src/app_service/service/wallet_connect/service.nim index a5f26f573c..ae3cadef73 100644 --- a/src/app_service/service/wallet_connect/service.nim +++ b/src/app_service/service/wallet_connect/service.nim @@ -123,11 +123,14 @@ QtObject: self.events.emit(SIGNAL_SHARED_KEYCARD_MODULE_AUTHENTICATE_USER, data) return true + proc signMessageUnsafe*(self: Service, address: string, password: string, message: string): string = + return status_go.signMessageUnsafe(address, password, message) + proc signMessage*(self: Service, address: string, password: string, message: string): string = return status_go.signMessage(address, password, message) - proc signTypedDataV4*(self: Service, address: string, password: string, typedDataJson: string): string = - return status_go.signTypedData(address, password, typedDataJson) + proc safeSignTypedData*(self: Service, address: string, password: string, typedDataJson: string, chainId: int, legacy: bool): string = + return status_go.safeSignTypedData(address, password, typedDataJson, chainId, legacy) proc signTransaction*(self: Service, address: string, chainId: int, password: string, txJson: string): string = var buildTxResponse: JsonNode diff --git a/src/backend/wallet_connect.nim b/src/backend/wallet_connect.nim index 232b83ed4a..985d0e3f93 100644 --- a/src/backend/wallet_connect.nim +++ b/src/backend/wallet_connect.nim @@ -4,12 +4,11 @@ import core, response_type import strutils from gen import rpc -import backend +import backend/wallet import status_go import app_service/service/community/dto/sign_params - import app_service/common/utils rpc(addWalletConnectSession, "wallet"): @@ -21,10 +20,15 @@ rpc(disconnectWalletConnectSession, "wallet"): rpc(getWalletConnectActiveSessions, "wallet"): validAtTimestamp: int -rpc(signTypedDataV4, "wallet"): +rpc(hashMessageEIP191, "wallet"): + message: string + +rpc(safeSignTypedDataForDApps, "wallet"): typedJson: string address: string password: string + chainId: int + legacy: bool proc isSuccessResponse(rpcResponse: RpcResponse[JsonNode]): bool = return rpcResponse.error.isNil @@ -34,7 +38,7 @@ proc addSession*(sessionJson: string): bool = let rpcRes = addWalletConnectSession(sessionJson) return isSuccessResponse(rpcRes): except Exception as e: - warn "AddWalletConnectSession failed: ", "msg", e.msg + error "AddWalletConnectSession failed: ", "msg", e.msg return false proc disconnectSession*(topic: string): bool = @@ -42,7 +46,7 @@ proc disconnectSession*(topic: string): bool = let rpcRes = disconnectWalletConnectSession(topic) return isSuccessResponse(rpcRes): except Exception as e: - warn "wallet_disconnectWalletConnectSession failed: ", "msg", e.msg + error "wallet_disconnectWalletConnectSession failed: ", "msg", e.msg return false proc getActiveSessions*(validAtTimestamp: int): JsonNode = @@ -61,7 +65,7 @@ proc getActiveSessions*(validAtTimestamp: int): JsonNode = return rpcRes.result except Exception as e: - warn "GetWalletConnectActiveSessions failed: ", "msg", e.msg + error "GetWalletConnectActiveSessions failed: ", "msg", e.msg return nil proc getDapps*(validAtEpoch: int64, testChains: bool): string = @@ -76,10 +80,10 @@ proc getDapps*(validAtEpoch: int64, testChains: bool): string = let jsonArray = $rpcRes.result return if jsonArray != "null": jsonArray else: "[]" except Exception as e: - warn "GetWalletConnectDapps failed: ", "msg", e.msg + error "GetWalletConnectDapps failed: ", "msg", e.msg return "" -proc signMessage*(address: string, password: string, message: string): string = +proc signMessageUnsafe*(address: string, password: string, message: string): string = try: let signParams = SignParamsDto(address: address, password: hashPassword(password), data: "0x" & toHex(message)) let paramsStr = $toJson(signParams) @@ -87,20 +91,37 @@ proc signMessage*(address: string, password: string, message: string): string = let rpcRes = Json.decode(rpcResRaw, RpcResponse[JsonNode]) if(not rpcRes.error.isNil): - return "" + return "" return rpcRes.result.getStr() except Exception as e: - warn "status_go.signMessage failed: ", "msg", e.msg + error "status_go.signMessage failed: ", "msg", e.msg return "" -proc signTypedData*(address: string, password: string, typedDataJson: string): string = +proc signMessage*(address: string, password: string, message: string): string = try: - let rpcRes = signTypedDataV4(typedDataJson, address, hashPassword(password)) + let hashRes = hashMessageEIP191("0x" & toHex(message)) + if not isSuccessResponse(hashRes): + error "wallet_hashMessageEIP191 failed: ", "msg", hashRes.error.message + return "" + let safeHash = hashRes.result.getStr() + let signRes = wallet.signMessage(safeHash, address, hashPassword(password)) + if not isSuccessResponse(signRes): + error "wallet_signMessage failed: ", "msg", signRes.error.message + return "" + + return signRes.result.getStr() + except Exception as e: + error "signMessageForDApps failed: ", "msg", e.msg + return "" + +proc safeSignTypedData*(address: string, password: string, typedDataJson: string, chainId: int, legacy: bool): string = + try: + let rpcRes = safeSignTypedDataForDApps(typedDataJson, address, hashPassword(password), chainId, legacy) if not isSuccessResponse(rpcRes): return "" return rpcRes.result.getStr() except Exception as e: - warn "wallet_signTypedDataV4 failed: ", "msg", e.msg + error (if legacy: "wallet_safeSignTypedDataForDApps" else: "wallet_signTypedDataV4") & " failed: ", "msg", e.msg return "" diff --git a/storybook/pages/DAppsWorkflowPage.qml b/storybook/pages/DAppsWorkflowPage.qml index e30bf0cfe3..b154f9d4cb 100644 --- a/storybook/pages/DAppsWorkflowPage.qml +++ b/storybook/pages/DAppsWorkflowPage.qml @@ -344,6 +344,12 @@ Item { return true } + // hardcoded for https://react-app.walletconnect.com/ + function signMessageUnsafe(topic, id, address, password, message) { + console.info(`calling mocked DAppsStore.signMessageUnsafe(${topic}, ${id}, ${address}, ${password}, ${message})`) + return "0xc8f39cb4cffa5c4659e0ccc7c417cc61d0cfc9e59de310368ac734065164f5515bfbaf4550d409896f7e2210b82a1cf65edcd77f696b4d3d24477fb81a90af8a1c" + } + // hardcoded for https://react-app.walletconnect.com/ function signMessage(topic, id, address, password, message) { console.info(`calling mocked DAppsStore.signMessage(${topic}, ${id}, ${address}, ${password}, ${message})`) @@ -351,8 +357,8 @@ Item { } // hardcoded for https://react-app.walletconnect.com/ - function signTypedDataV4(topic, id, address, password, typedDataJson) { - console.info(`calling mocked DAppsStore.signTypedDataV4(${topic}, ${id}, ${address}, ${password}, ${typedDataJson})`) + function safeSignTypedData(topic, id, address, password, typedDataJson, chainId, legacy) { + console.info(`calling mocked DAppsStore.safeSignTypedData(${topic}, ${id}, ${address}, ${password}, ${typedDataJson}, ${chainId}, ${legacy})`) return "0xf8ceb3468319cc215523b67c24c4504b3addd9bf8de31c278038d7478c9b6de554f7d8a516cd5d6a066b7d48b81f03d9d6bb7d5d754513c08325674ebcc7efbc1b" } diff --git a/storybook/qmlTests/tests/tst_DAppsWorkflow.qml b/storybook/qmlTests/tests/tst_DAppsWorkflow.qml index 00a4092c01..7e72901d9d 100644 --- a/storybook/qmlTests/tests/tst_DAppsWorkflow.qml +++ b/storybook/qmlTests/tests/tst_DAppsWorkflow.qml @@ -106,9 +106,9 @@ Item { function signMessage(topic, id, address, password, message) { signMessageCalls.push({topic, id, address, password, message}) } - property var signTypedDataV4Calls: [] - function signTypedDataV4(topic, id, address, password, message) { - signTypedDataV4Calls.push({topic, id, address, password, message}) + property var safeSignTypedDataCalls: [] + function safeSignTypedData(topic, id, address, password, message, chainId, legacy) { + safeSignTypedDataCalls.push({topic, id, address, password, message, chainId, legacy}) } property var updateWalletConnectSessionsCalls: [] diff --git a/ui/app/AppLayouts/Wallet/services/dapps/DAppsRequestHandler.qml b/ui/app/AppLayouts/Wallet/services/dapps/DAppsRequestHandler.qml index ae67374cfa..3be258f1e4 100644 --- a/ui/app/AppLayouts/Wallet/services/dapps/DAppsRequestHandler.qml +++ b/ui/app/AppLayouts/Wallet/services/dapps/DAppsRequestHandler.qml @@ -178,7 +178,14 @@ QObject { return null } address = event.params.request.params[1] - } else if(method === SessionRequest.methods.signTypedData_v4.name) { + } else if (method === SessionRequest.methods.sign.name) { + if (event.params.request.params.length === 1) { + return null + } + address = event.params.request.params[0] + } else if(method === SessionRequest.methods.signTypedData_v4.name || + method === SessionRequest.methods.signTypedData.name) + { if (event.params.request.params.length < 2) { return null } @@ -205,12 +212,15 @@ QObject { } function extractMethodData(event, method) { - if (method === SessionRequest.methods.personalSign.name) { - if (event.params.request.params.length == 0) { + if (method === SessionRequest.methods.personalSign.name || + method === SessionRequest.methods.sign.name) + { + if (event.params.request.params.length < 1) { return null } var message = "" - let messageParam = event.params.request.params[0] + let messageIndex = (method === SessionRequest.methods.personalSign.name ? 0 : 1) + let messageParam = event.params.request.params[messageIndex] // There is no standard on how data is encoded. Therefore we support hex or utf8 if (Helpers.isHex(messageParam)) { message = Helpers.hexToString(messageParam) @@ -218,12 +228,17 @@ QObject { message = messageParam } return SessionRequest.methods.personalSign.buildDataObject(message) - } else if (method === SessionRequest.methods.signTypedData_v4.name) { + } else if (method === SessionRequest.methods.signTypedData_v4.name || + method === SessionRequest.methods.signTypedData.name) + { if (event.params.request.params.length < 2) { return null } let jsonMessage = event.params.request.params[1] - return SessionRequest.methods.signTypedData_v4.buildDataObject(jsonMessage) + let methodObj = method === SessionRequest.methods.signTypedData_v4.name + ? SessionRequest.methods.signTypedData_v4 + : SessionRequest.methods.signTypedData + return methodObj.buildDataObject(jsonMessage) } else if (method === SessionRequest.methods.signTransaction.name) { if (event.params.request.params.length == 0) { return null @@ -260,16 +275,22 @@ QObject { if (password !== "") { var actionResult = "" - if (request.method === SessionRequest.methods.personalSign.name) { - // TODO #14756: clarify why prefixing the message fails the test app https://react-app.walletconnect.com/ - //let finalMessage = "\x19Ethereum Signed Message:\n" + originalMessage.length + originalMessage + if (request.method === SessionRequest.methods.sign.name) { + actionResult = store.signMessageUnsafe(request.topic, request.id, + request.account.address, password, + SessionRequest.methods.personalSign.getMessageFromData(request.data)) + } else if (request.method === SessionRequest.methods.personalSign.name) { actionResult = store.signMessage(request.topic, request.id, request.account.address, password, SessionRequest.methods.personalSign.getMessageFromData(request.data)) - } else if (request.method === SessionRequest.methods.signTypedData_v4.name) { - actionResult = store.signTypedDataV4(request.topic, request.id, + } else if (request.method === SessionRequest.methods.signTypedData_v4.name || + request.method === SessionRequest.methods.signTypedData.name) + { + let legacy = request.method === SessionRequest.methods.signTypedData.name + actionResult = store.safeSignTypedData(request.topic, request.id, request.account.address, password, - SessionRequest.methods.signTypedData_v4.getMessageFromData(request.data)) + SessionRequest.methods.signTypedData.getMessageFromData(request.data), + request.network.chainId, legacy) } else if (request.method === SessionRequest.methods.signTransaction.name) { let txObj = SessionRequest.methods.signTransaction.getTxObjFromData(request.data) actionResult = store.signTransaction(request.topic, request.id, diff --git a/ui/app/AppLayouts/Wallet/services/dapps/types/SessionRequest.qml b/ui/app/AppLayouts/Wallet/services/dapps/types/SessionRequest.qml index fd7496aff7..9996571306 100644 --- a/ui/app/AppLayouts/Wallet/services/dapps/types/SessionRequest.qml +++ b/ui/app/AppLayouts/Wallet/services/dapps/types/SessionRequest.qml @@ -17,6 +17,14 @@ QtObject { function buildDataObject(message) { return {message} } function getMessageFromData(data) { return data.message } } + readonly property QtObject sign: QtObject { + readonly property string name: "eth_sign" + readonly property string userString: qsTr("sign") + readonly property string requestDisplay: qsTr("sign this message") + + function buildDataObject(message) { return {message} } + function getMessageFromData(data) { return data.message } + } readonly property QtObject signTypedData_v4: QtObject { readonly property string name: "eth_signTypedData_v4" readonly property string userString: qsTr("sign typed data") @@ -25,7 +33,14 @@ QtObject { function buildDataObject(message) { return {message} } function getMessageFromData(data) { return data.message } } + readonly property QtObject signTypedData: QtObject { + readonly property string name: "eth_signTypedData" + readonly property string userString: qsTr("sign typed data") + readonly property string requestDisplay: qsTr("sign this message") + function buildDataObject(message) { return {message} } + function getMessageFromData(data) { return data.message } + } readonly property QtObject signTransaction: QtObject { readonly property string name: "eth_signTransaction" readonly property string userString: qsTr("sign transaction") @@ -43,7 +58,7 @@ QtObject { function buildDataObject(tx) { return {tx}} function getTxObjFromData(data) { return data.tx } } - readonly property var all: [personalSign, signTypedData_v4, signTransaction, sendTransaction] + readonly property var all: [personalSign, sign, signTypedData_v4, signTypedData, signTransaction, sendTransaction] } function getSupportedMethods() { diff --git a/ui/imports/shared/popups/walletconnect/DAppRequestModal.qml b/ui/imports/shared/popups/walletconnect/DAppRequestModal.qml index df58361385..fb8b4969bf 100644 --- a/ui/imports/shared/popups/walletconnect/DAppRequestModal.qml +++ b/ui/imports/shared/popups/walletconnect/DAppRequestModal.qml @@ -327,12 +327,23 @@ StatusDialog { userDisplayNaming = SessionRequest.methods.personalSign.requestDisplay break } + case SessionRequest.methods.sign.name: { + payloadToDisplay = SessionRequest.methods.sign.getMessageFromData(root.payloadData) + userDisplayNaming = SessionRequest.methods.sign.requestDisplay + break + } case SessionRequest.methods.signTypedData_v4.name: { let messageObject = SessionRequest.methods.signTypedData_v4.getMessageFromData(root.payloadData) payloadToDisplay = JSON.stringify(JSON.parse(messageObject), null, 2) userDisplayNaming = SessionRequest.methods.signTypedData_v4.requestDisplay break } + case SessionRequest.methods.signTypedData.name: { + let messageObject = SessionRequest.methods.signTypedData.getMessageFromData(root.payloadData) + payloadToDisplay = JSON.stringify(JSON.parse(messageObject), null, 2) + userDisplayNaming = SessionRequest.methods.signTypedData.requestDisplay + break + } case SessionRequest.methods.signTransaction.name: { let tx = SessionRequest.methods.signTransaction.getTxObjFromData(root.payloadData) payloadToDisplay = JSON.stringify(tx, null, 2) diff --git a/ui/imports/shared/stores/DAppsStore.qml b/ui/imports/shared/stores/DAppsStore.qml index a4d703c638..f2c408a2b8 100644 --- a/ui/imports/shared/stores/DAppsStore.qml +++ b/ui/imports/shared/stores/DAppsStore.qml @@ -31,14 +31,19 @@ QObject { } } + // Returns the hex encoded signature of the message or empty string if error + function signMessageUnsafe(topic, id, address, password, message) { + return controller.signMessageUnsafe(address, password, message) + } + // Returns the hex encoded signature of the message or empty string if error function signMessage(topic, id, address, password, message) { return controller.signMessage(address, password, message) } // Returns the hex encoded signature of the typedDataJson or empty string if error - function signTypedDataV4(topic, id, address, password, typedDataJson) { - return controller.signTypedDataV4(address, password, typedDataJson) + function safeSignTypedData(topic, id, address, password, typedDataJson, chainId, legacy) { + return controller.safeSignTypedData(address, password, typedDataJson, chainId, legacy) } // Remove leading zeros from hex number as expected by status-go diff --git a/vendor/status-go b/vendor/status-go index 3145ab05ff..5336c47f1b 160000 --- a/vendor/status-go +++ b/vendor/status-go @@ -1 +1 @@ -Subproject commit 3145ab05ff5a3cbd56c9c8f9db76352d474b464c +Subproject commit 5336c47f1b28b1d1ee8ef5e2b1bbf9bdc479f409