Skip to content

Commit

Permalink
fix: adjust Ledger signatures for notifications
Browse files Browse the repository at this point in the history
  • Loading branch information
iamacook committed Sep 26, 2023
1 parent 508efd3 commit 8ae0a46
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { PUSH_NOTIFICATION_EVENTS } from '@/services/analytics/events/push-notif
import { getRegisterDevicePayload } from '../logic'
import { logError } from '@/services/exceptions'
import ErrorCodes from '@/services/exceptions/ErrorCodes'
import useWallet from '@/hooks/wallets/useWallet'
import { isLedger } from '@/utils/wallets'
import type { NotifiableSafes } from '../logic'

const registrationFlow = async (registrationFn: Promise<unknown>, callback: () => void): Promise<boolean> => {
Expand Down Expand Up @@ -38,11 +40,12 @@ export const useNotificationRegistrations = (): {
} => {
const dispatch = useAppDispatch()
const web3 = useWeb3()
const wallet = useWallet()

const { uuid, _createPreferences, _deletePreferences, _deleteAllPreferences } = useNotificationPreferences()

const registerNotifications = async (safesToRegister: NotifiableSafes) => {
if (!uuid || !web3) {
if (!uuid || !web3 || !wallet) {
return
}

Expand All @@ -51,6 +54,7 @@ export const useNotificationRegistrations = (): {
uuid,
safesToRegister,
web3,
isLedger: isLedger(wallet),
})

return registerDevice(payload)
Expand Down
83 changes: 79 additions & 4 deletions src/components/settings/PushNotifications/logic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ Object.defineProperty(globalThis, 'location', {
},
})

const MM_SIGNATURE =
'0x844ba559793a122c5742e9d922ed1f4650d4efd8ea35191105ddaee6a604000165c14f56278bda8d52c9400cdaeaf5cdc38d3596264cc5ccd8f03e5619d5d9d41b'
const LEDGER_SIGNATURE =
'0xb1274687aea0d8b8578a3eb6da57979eee0a64225e04445a0858e6f8d0d1b5870cdff961513992d849e47e9b0a8d432019829f1e4958837fd86e034656766a4e00'
const ADJUSTED_LEDGER_SIGNATURE =
'0xb1274687aea0d8b8578a3eb6da57979eee0a64225e04445a0858e6f8d0d1b5870cdff961513992d849e47e9b0a8d432019829f1e4958837fd86e034656766a4e1b'

describe('Notifications', () => {
let alertMock = jest.fn()

Expand Down Expand Up @@ -88,18 +95,85 @@ describe('Notifications', () => {
})
})

describe('adjustLegerSignature', () => {
it('should return the same signature if not that of a Ledger', () => {
const adjustedSignature = logic._adjustLedgerSignatureV(MM_SIGNATURE)

expect(adjustedSignature).toBe(MM_SIGNATURE)
})

it('should return an adjusted signature if is that of a Ledger and v is 0 or 1', () => {
const adjustedSignature = logic._adjustLedgerSignatureV(LEDGER_SIGNATURE)

expect(adjustedSignature).toBe(ADJUSTED_LEDGER_SIGNATURE)
})

it('should return the same signature if v is 27 or 28', () => {
const adjustedSignature = logic._adjustLedgerSignatureV(MM_SIGNATURE)

expect(adjustedSignature).toBe(MM_SIGNATURE)
})
})

describe('getRegisterDevicePayload', () => {
it('should return the payload with signature', async () => {
const token = crypto.randomUUID()
jest.spyOn(firebase, 'getToken').mockImplementation(() => Promise.resolve(token))

const mockProvider = new Web3Provider(jest.fn())
const signature = hexZeroPad('0x69420', 65)

jest.spyOn(mockProvider, 'getSigner').mockImplementation(
() =>
({
signMessage: jest.fn().mockResolvedValueOnce(signature),
signMessage: jest.fn().mockResolvedValueOnce(MM_SIGNATURE),
} as unknown as JsonRpcSigner),
)

const uuid = crypto.randomUUID()

const payload = await logic.getRegisterDevicePayload({
safesToRegister: {
['1']: [hexZeroPad('0x1', 20), hexZeroPad('0x2', 20)],
['2']: [hexZeroPad('0x1', 20)],
},
uuid,
web3: mockProvider,
isLedger: false,
})

expect(payload).toStrictEqual({
uuid,
cloudMessagingToken: token,
buildNumber: '0',
bundle: 'safe',
deviceType: DeviceType.WEB,
version: packageJson.version,
timestamp: expect.any(String),
safeRegistrations: [
{
chainId: '1',
safes: [hexZeroPad('0x1', 20), hexZeroPad('0x2', 20)],
signatures: [MM_SIGNATURE],
},
{
chainId: '2',
safes: [hexZeroPad('0x1', 20)],
signatures: [MM_SIGNATURE],
},
],
})
})

it('should return the payload with a Ledger adjusted signature', async () => {
const token = crypto.randomUUID()
jest.spyOn(firebase, 'getToken').mockImplementation(() => Promise.resolve(token))

const mockProvider = new Web3Provider(jest.fn())

jest.spyOn(mockProvider, 'getSigner').mockImplementation(
() =>
({
signMessage: jest.fn().mockResolvedValueOnce(LEDGER_SIGNATURE),
} as unknown as JsonRpcSigner),
)

Expand All @@ -112,6 +186,7 @@ describe('Notifications', () => {
},
uuid,
web3: mockProvider,
isLedger: true,
})

expect(payload).toStrictEqual({
Expand All @@ -126,12 +201,12 @@ describe('Notifications', () => {
{
chainId: '1',
safes: [hexZeroPad('0x1', 20), hexZeroPad('0x2', 20)],
signatures: [signature],
signatures: [ADJUSTED_LEDGER_SIGNATURE],
},
{
chainId: '2',
safes: [hexZeroPad('0x1', 20)],
signatures: [signature],
signatures: [ADJUSTED_LEDGER_SIGNATURE],
},
],
})
Expand Down
31 changes: 28 additions & 3 deletions src/components/settings/PushNotifications/logic.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { arrayify, keccak256, toUtf8Bytes } from 'ethers/lib/utils'
import { arrayify, joinSignature, keccak256, splitSignature, toUtf8Bytes } from 'ethers/lib/utils'
import { getToken, getMessaging } from 'firebase/messaging'
import { DeviceType } from '@safe-global/safe-gateway-typescript-sdk'
import type { RegisterNotificationsRequest } from '@safe-global/safe-gateway-typescript-sdk'
Expand Down Expand Up @@ -31,18 +31,34 @@ export const requestNotificationPermission = async (): Promise<boolean> => {
return permission === 'granted'
}

const getSafeRegistrationSignature = ({
// Ledger produces vrs signatures with a canonical v value of {0,1}
// Ethereum's ecrecover call only accepts a non-standard v value of {27,28}.

// @see https://github.com/ethereum/go-ethereum/issues/19751
export const _adjustLedgerSignatureV = (signature: string): string => {
const split = splitSignature(signature)

if (split.v === 0 || split.v === 1) {
split.v += 27
}

return joinSignature(split)
}

const getSafeRegistrationSignature = async ({
safeAddresses,
web3,
timestamp,
uuid,
token,
isLedger,
}: {
safeAddresses: Array<string>
web3: Web3Provider
timestamp: string
uuid: string
token: string
isLedger: boolean
}) => {
const MESSAGE_PREFIX = 'gnosis-safe'

Expand All @@ -55,7 +71,13 @@ const getSafeRegistrationSignature = ({
const message = MESSAGE_PREFIX + timestamp + uuid + token + safeAddresses.sort().join('')
const hashedMessage = keccak256(toUtf8Bytes(message))

return web3.getSigner().signMessage(arrayify(hashedMessage))
const signature = await web3.getSigner().signMessage(arrayify(hashedMessage))

if (!isLedger) {
return signature
}

return _adjustLedgerSignatureV(signature)
}

export type NotifiableSafes = { [chainId: string]: Array<string> }
Expand All @@ -64,10 +86,12 @@ export const getRegisterDevicePayload = async ({
safesToRegister,
uuid,
web3,
isLedger,
}: {
safesToRegister: NotifiableSafes
uuid: string
web3: Web3Provider
isLedger: boolean
}): Promise<RegisterNotificationsRequest> => {
const BUILD_NUMBER = '0' // Required value, but does not exist on web
const BUNDLE = 'safe'
Expand Down Expand Up @@ -101,6 +125,7 @@ export const getRegisterDevicePayload = async ({
uuid,
timestamp,
token,
isLedger,
})

return {
Expand Down
4 changes: 4 additions & 0 deletions src/utils/wallets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ export const isWalletRejection = (err: EthersError | Error): boolean => {
return isEthersRejection(err as EthersError) || isWCRejection(err)
}

export const isLedger = (wallet: ConnectedWallet): boolean => {
return wallet.label.toUpperCase() === WALLET_KEYS.LEDGER
}

export const isHardwareWallet = (wallet: ConnectedWallet): boolean => {
return [WALLET_KEYS.LEDGER, WALLET_KEYS.TREZOR, WALLET_KEYS.KEYSTONE].includes(
wallet.label.toUpperCase() as WALLET_KEYS,
Expand Down

0 comments on commit 8ae0a46

Please sign in to comment.