From d2d4ef2b35afe72989370444267c69d9adabe63b Mon Sep 17 00:00:00 2001 From: skylarbarrera Date: Wed, 25 Oct 2023 15:11:18 -0400 Subject: [PATCH 1/4] util --- src/utils/signingUtils.ts | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 src/utils/signingUtils.ts diff --git a/src/utils/signingUtils.ts b/src/utils/signingUtils.ts new file mode 100644 index 00000000000..ab09bf216a6 --- /dev/null +++ b/src/utils/signingUtils.ts @@ -0,0 +1,35 @@ +// This function removes all the keys from the message that are not present in the types +// preventing a know phising attack where the signature process could allow malicious DApps +// to trick users into signing an EIP-712 object different from the one presented +// in the signature approval preview. Consequently, users were at risk of unknowingly +// transferring control of their ERC-20 tokens, NFTs, etc to adversaries by signing +// hidden Permit messages. + +// For more info read https://www.coinspect.com/wallet-EIP-712-injection-vulnerability/ + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export const sanitizeTypedData = (data: any) => { + if (data.types[data.primaryType].length > 0) { + // Extract all the valid permit types for the primary type + const permitPrimaryTypes: string[] = data.types[data.primaryType].map( + (type: { name: string; type: string }) => type.name + ); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const sanitizedMessage: any = {}; + // Extract all the message keys that matches the valid permit types + Object.keys(data.message).forEach(key => { + if (permitPrimaryTypes.includes(key)) { + sanitizedMessage[key] = data.message[key]; + } + }); + + const sanitizedData = { + ...data, + message: sanitizedMessage, + }; + + return sanitizedData; + } + return data; +}; From c6342bc675c6f8cdfd558415d285e3362da97554 Mon Sep 17 00:00:00 2001 From: skylarbarrera Date: Wed, 25 Oct 2023 15:11:38 -0400 Subject: [PATCH 2/4] tx signatures --- src/model/wallet.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/model/wallet.ts b/src/model/wallet.ts index 7909b987fcc..e885f371241 100644 --- a/src/model/wallet.ts +++ b/src/model/wallet.ts @@ -72,6 +72,7 @@ import { import { DebugContext } from '@/logger/debugContext'; import { IS_ANDROID } from '@/env'; import { setHardwareTXError } from '@/navigation/HardwareWalletTxNavigator'; +import { sanitizeTypedData } from '@/utils/signingUtils'; export type EthereumPrivateKey = string; type EthereumMnemonic = string; @@ -516,7 +517,7 @@ export const signTypedDataMessage = async ( isHardwareWallet = true; } try { - let parsedData = message; + let parsedData = sanitizeTypedData(message); try { parsedData = typeof message === 'string' && JSON.parse(message); // eslint-disable-next-line no-empty From e79856b07730ba9ae87caca93cb1800155fbe2c0 Mon Sep 17 00:00:00 2001 From: skylarbarrera Date: Wed, 25 Oct 2023 15:11:53 -0400 Subject: [PATCH 3/4] display --- src/components/transaction/TransactionMessage.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/transaction/TransactionMessage.js b/src/components/transaction/TransactionMessage.js index 3441b546d27..45ced2bcde4 100644 --- a/src/components/transaction/TransactionMessage.js +++ b/src/components/transaction/TransactionMessage.js @@ -7,6 +7,7 @@ import { Text } from '../text'; import styled from '@/styled-thing'; import { padding } from '@/styles'; import { deviceUtils } from '@/utils'; +import { sanitizeTypedData } from '@/utils/signingUtils'; const deviceWidth = deviceUtils.dimensions.width; const horizontalPadding = 24; @@ -34,7 +35,8 @@ const TransactionMessage = ({ maxHeight = 150, message, method }) => { maximumHeight = 200; minimumHeight = 200; try { - msg = JSON.parse(message); + const sanitizedMessage = sanitizeTypedData(message); + msg = JSON.parse(sanitizedMessage); // eslint-disable-next-line no-empty } catch (e) {} msg = JSON.stringify(msg, null, 4); From 8d0c2f6f834794f474299572fb50fe86eb9625f0 Mon Sep 17 00:00:00 2001 From: skylarbarrera Date: Mon, 30 Oct 2023 12:49:37 -0400 Subject: [PATCH 4/4] fix parsing --- src/components/transaction/TransactionMessage.js | 15 ++++++++++++--- src/model/wallet.ts | 9 +++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/components/transaction/TransactionMessage.js b/src/components/transaction/TransactionMessage.js index 45ced2bcde4..a2ec6d6f67d 100644 --- a/src/components/transaction/TransactionMessage.js +++ b/src/components/transaction/TransactionMessage.js @@ -8,6 +8,7 @@ import styled from '@/styled-thing'; import { padding } from '@/styles'; import { deviceUtils } from '@/utils'; import { sanitizeTypedData } from '@/utils/signingUtils'; +import { RainbowError, logger } from '@/logger'; const deviceWidth = deviceUtils.dimensions.width; const horizontalPadding = 24; @@ -35,10 +36,18 @@ const TransactionMessage = ({ maxHeight = 150, message, method }) => { maximumHeight = 200; minimumHeight = 200; try { - const sanitizedMessage = sanitizeTypedData(message); - msg = JSON.parse(sanitizedMessage); + const parsedMessage = JSON.parse(message); + const sanitizedMessage = sanitizeTypedData(parsedMessage); + msg = sanitizedMessage; // eslint-disable-next-line no-empty - } catch (e) {} + } catch (e) { + logger.error( + new RainbowError('TransactionMessage: Error parsing message ', { + messageType: typeof message, + }) + ); + } + msg = JSON.stringify(msg, null, 4); } diff --git a/src/model/wallet.ts b/src/model/wallet.ts index e885f371241..a9a647a673e 100644 --- a/src/model/wallet.ts +++ b/src/model/wallet.ts @@ -517,9 +517,14 @@ export const signTypedDataMessage = async ( isHardwareWallet = true; } try { - let parsedData = sanitizeTypedData(message); + let parsedData = message; + + // we need to parse the data different for both possible types try { - parsedData = typeof message === 'string' && JSON.parse(message); + parsedData = + typeof message === 'string' + ? sanitizeTypedData(JSON.parse(message)) + : sanitizeTypedData(message); // eslint-disable-next-line no-empty } catch (e) {}