Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: restrict safe pass safe app access for ofac blocked addresses #4066

Merged
merged 8 commits into from
Oct 14, 2024
12 changes: 10 additions & 2 deletions src/components/common/BlockedAddress/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@ import { useRouter } from 'next/router'
import Disclaimer from '@/components/common/Disclaimer'
import { AppRoutes } from '@/config/routes'

export const BlockedAddress = ({ address, featureTitle }: { address: string; featureTitle: string }): ReactElement => {
export const BlockedAddress = ({
address,
featureTitle,
onClose,
}: {
address: string
featureTitle: string
onClose?: () => void
}): ReactElement => {
const theme = useTheme()
const isMobile = useMediaQuery(theme.breakpoints.down('sm'))
const displayAddress = address && isMobile ? shortenAddress(address) : address
Expand All @@ -20,7 +28,7 @@ export const BlockedAddress = ({ address, featureTitle }: { address: string; fea
title="Blocked address"
subtitle={displayAddress}
content={`The above address is part of the OFAC SDN list and the ${featureTitle} is unavailable for sanctioned addresses.`}
onAccept={handleAccept}
onAccept={onClose ?? handleAccept}
/>
)
}
Expand Down
4 changes: 3 additions & 1 deletion src/components/common/SafeTokenWidget/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Track from '../Track'
import SafeTokenIcon from '@/public/images/common/safe-token.svg'
import SafePassStar from '@/public/images/common/safe-pass-star.svg'
import css from './styles.module.css'
import { useSanctionedAddress } from '@/hooks/useSanctionedAddress'
import useSafeAddress from '@/hooks/useSafeAddress'
import { skipToken } from '@reduxjs/toolkit/query/react'
import { useDarkMode } from '@/hooks/useDarkMode'
Expand Down Expand Up @@ -43,13 +44,14 @@ const SafeTokenWidget = () => {
const [allocationData, , allocationDataLoading] = useSafeTokenAllocation()
const [allocation, , allocationLoading] = useSafeVotingPower(allocationData)

const sanctionedAddress = useSanctionedAddress()
const { data: ownGlobalRank, isLoading: ownGlobalRankLoading } = useGetOwnGlobalCampaignRankQuery(
chainId !== '1' && chainId !== '11155111' ? skipToken : { chainId, safeAddress },
{ refetchOnFocus: false },
)

const tokenAddress = getSafeTokenAddress(chainId)
if (!tokenAddress) {
if (!tokenAddress || Boolean(sanctionedAddress)) {
return null
}

Expand Down
6 changes: 5 additions & 1 deletion src/components/dashboard/ActivityRewardsSection/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { useHasFeature } from '@/hooks/useChains'
import { FEATURES } from '@/utils/chains'
import useLocalStorage from '@/services/local-storage/useLocalStorage'
import ExternalLink from '@/components/common/ExternalLink'
import { useSanctionedAddress } from '@/hooks/useSanctionedAddress'

const Step = ({ active, title }: { active: boolean; title: ReactNode }) => {
return (
Expand Down Expand Up @@ -48,7 +49,10 @@ const ActivityRewardsSection = () => {
const isSAPBannerEnabled = useHasFeature(FEATURES.SAP_BANNER)
const governanceApp = matchingApps?.[0]

if (!governanceApp || !governanceApp?.url || !isSAPBannerEnabled || widgetHidden) return null
const sanctionedAddress = useSanctionedAddress(isSAPBannerEnabled && !widgetHidden && !!governanceApp)

if (!governanceApp || !governanceApp?.url || !isSAPBannerEnabled || widgetHidden || Boolean(sanctionedAddress))
return null

const appUrl = getSafeAppUrl(router, governanceApp?.url)

Expand Down
22 changes: 21 additions & 1 deletion src/components/safe-apps/AppFrame/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { type AddressBookItem, Methods } from '@safe-global/safe-apps-sdk'
import type { ReactElement } from 'react'
import { useMemo } from 'react'
import { useCallback, useEffect } from 'react'
import { CircularProgress, Typography } from '@mui/material'
import { Box, CircularProgress, Typography } from '@mui/material'
import { useRouter } from 'next/router'
import Head from 'next/head'
import type { RequestId } from '@safe-global/safe-apps-sdk'
Expand All @@ -28,6 +28,11 @@ import { PermissionStatus, type SafeAppDataWithPermissions } from '@/components/
import css from './styles.module.css'
import SafeAppIframe from './SafeAppIframe'
import { useCustomAppCommunicator } from '@/hooks/safe-apps/useCustomAppCommunicator'
import { useSanctionedAddress } from '@/hooks/useSanctionedAddress'
import BlockedAddress from '@/components/common/BlockedAddress'
import { isSafePassApp } from '@/features/walletconnect/services/utils'

const UNKNOWN_APP_NAME = 'Unknown Safe App'

type AppFrameProps = {
appUrl: string
Expand All @@ -42,6 +47,8 @@ const AppFrame = ({ appUrl, allowedFeaturesList, safeAppFromManifest, isNativeEm
const chainId = useChainId()
const chain = useCurrentChain()
const router = useRouter()
const isSafePass = isSafePassApp(appUrl)
const sanctionedAddress = useSanctionedAddress(isSafePass)
const {
expanded: queueBarExpanded,
dismissedByUser: queueBarDismissed,
Expand Down Expand Up @@ -118,6 +125,19 @@ const AppFrame = ({ appUrl, allowedFeaturesList, safeAppFromManifest, isNativeEm
return <div />
}

if (sanctionedAddress && isSafePass) {
return (
<>
<Head>
<title>{`Safe Apps - Viewer - ${remoteApp ? remoteApp.name : UNKNOWN_APP_NAME}`}</title>
</Head>
<Box p={2}>
<BlockedAddress address={sanctionedAddress} featureTitle="Safe{Pass} Safe app" />
</Box>
</>
)
}

return (
<>
{!isNativeEmbed && (
Expand Down
1 change: 1 addition & 0 deletions src/config/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,5 +107,6 @@ export const REDEFINE_ARTICLE = 'https://safe.mirror.xyz/rInLWZwD_sf7enjoFerj6FI

export const CHAINALYSIS_OFAC_CONTRACT = '0x40c57923924b5c5c5455c48d93317139addac8fb'

export const SAFE_PASS_URL = 'community.safe.global'
export const ECOSYSTEM_ID_ADDRESS =
process.env.NEXT_PUBLIC_ECOSYSTEM_ID_ADDRESS || '0x0000000000000000000000000000000000000000'
18 changes: 17 additions & 1 deletion src/features/walletconnect/components/WcProposalForm/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
getPeerName,
getSupportedChainIds,
isBlockedBridge,
isSafePassApp,
isWarnedBridge,
} from '@/features/walletconnect/services/utils'
import { WalletConnectContext } from '@/features/walletconnect/WalletConnectContext'
Expand All @@ -20,6 +21,8 @@ import { type Dispatch, type SetStateAction, useCallback, useContext, useEffect,
import { CompatibilityWarning } from './CompatibilityWarning'
import ProposalVerification from './ProposalVerification'
import css from './styles.module.css'
import { useSanctionedAddress } from '@/hooks/useSanctionedAddress'
import BlockedAddress from '@/components/common/BlockedAddress'

type ProposalFormProps = {
proposal: Web3WalletTypes.SessionProposal
Expand All @@ -38,13 +41,22 @@ const WcProposalForm = ({ proposal, setProposal, onApprove }: ProposalFormProps)
const { isScam, origin } = proposal.verifyContext.verified
const url = proposer.metadata.url || origin

const isSafePass = isSafePassApp(origin)
const sanctionedAddress = useSanctionedAddress(isSafePass)

const chainIds = useMemo(() => getSupportedChainIds(configs, proposal.params), [configs, proposal.params])
const isUnsupportedChain = !chainIds.includes(chainId)

const name = getPeerName(proposer) || 'Unknown dApp'
const isHighRisk = proposal.verifyContext.verified.validation === 'INVALID' || isWarnedBridge(origin, name)
const isBlocked = isScam || isBlockedBridge(origin)
const disabled = !safeLoaded || isUnsupportedChain || isBlocked || (isHighRisk && !understandsRisk) || !!isLoading
const disabled =
!safeLoaded ||
isUnsupportedChain ||
isBlocked ||
(isHighRisk && !understandsRisk) ||
!!isLoading ||
(Boolean(sanctionedAddress) && isSafePass)

// On session reject
const onReject = useCallback(async () => {
Expand Down Expand Up @@ -134,6 +146,10 @@ const WcProposalForm = ({ proposal, setProposal, onApprove }: ProposalFormProps)
/>
)}

{isSafePass && sanctionedAddress && (
<BlockedAddress address={sanctionedAddress} featureTitle="Safe{Pass}" onClose={onReject} />
)}

<Divider flexItem className={css.divider} />

<div className={css.buttons}>
Expand Down
4 changes: 4 additions & 0 deletions src/features/walletconnect/services/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ export const isBlockedBridge = (origin: string) => {
return BlockedBridges.some((bridge) => origin.includes(bridge))
}

export const isSafePassApp = (origin: string) => {
return origin.includes('community.safe.global')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we if not avoid hardcode then at least make it a constant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the constant SAFE_PASS_URL here?

}

// Bridge defaults to same address on destination chain but allows changing it
export const isWarnedBridge = (origin: string, name: string) => {
return WarnedBridges.some((bridge) => origin.includes(bridge)) || WarnedBridgeNames.includes(name)
Expand Down
92 changes: 92 additions & 0 deletions src/hooks/__tests__/useSanctionedAddress.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { renderHook } from '@/tests/test-utils'
import { useSanctionedAddress } from '../useSanctionedAddress'
import useSafeAddress from '../useSafeAddress'
import useWallet from '../wallets/useWallet'
import { faker } from '@faker-js/faker'
import { connectedWalletBuilder } from '@/tests/builders/wallet'
import * as ofac from '@/store/ofac'
import { skipToken } from '@reduxjs/toolkit/query'

jest.mock('@/hooks/useSafeAddress')
jest.mock('@/hooks/wallets/useWallet')

describe('useSanctionedAddress', () => {
const mockUseSafeAddress = useSafeAddress as jest.MockedFunction<typeof useSafeAddress>
const mockUseWallet = useWallet as jest.MockedFunction<typeof useWallet>

it('should return undefined without safeAddress and wallet', () => {
const { result } = renderHook(() => useSanctionedAddress())
expect(result.current).toBeUndefined()
})

it('should return undefined if neither safeAddress nor wallet are sanctioned', () => {
mockUseSafeAddress.mockReturnValue(faker.finance.ethereumAddress())
mockUseWallet.mockReturnValue(connectedWalletBuilder().build())

jest.spyOn(ofac, 'useGetIsSanctionedQuery').mockReturnValue({ data: false, refetch: jest.fn() })

const { result } = renderHook(() => useSanctionedAddress())
expect(result.current).toBeUndefined()
})

it('should return safeAddress if it is sanctioned', () => {
const mockSafeAddress = faker.finance.ethereumAddress()
const mockWalletAddress = faker.finance.ethereumAddress()
mockUseSafeAddress.mockReturnValue(mockSafeAddress)
mockUseWallet.mockReturnValue(connectedWalletBuilder().with({ address: mockWalletAddress }).build())

jest
.spyOn(ofac, 'useGetIsSanctionedQuery')
.mockImplementation((address) => ({ data: address === mockSafeAddress, refetch: jest.fn() }))

const { result } = renderHook(() => useSanctionedAddress())
expect(result.current).toEqual(mockSafeAddress)
})

it('should return walletAddress if it is sanctioned', () => {
const mockSafeAddress = faker.finance.ethereumAddress()
const mockWalletAddress = faker.finance.ethereumAddress()
mockUseSafeAddress.mockReturnValue(mockSafeAddress)
mockUseWallet.mockReturnValue(connectedWalletBuilder().with({ address: mockWalletAddress }).build())

jest
.spyOn(ofac, 'useGetIsSanctionedQuery')
.mockImplementation((address) => ({ data: address === mockWalletAddress, refetch: jest.fn() }))

const { result } = renderHook(() => useSanctionedAddress())
expect(result.current).toEqual(mockWalletAddress)
})

it('should return safeAddress if both are sanctioned', () => {
const mockSafeAddress = faker.finance.ethereumAddress()
const mockWalletAddress = faker.finance.ethereumAddress()
mockUseSafeAddress.mockReturnValue(mockSafeAddress)
mockUseWallet.mockReturnValue(connectedWalletBuilder().with({ address: mockWalletAddress }).build())

jest.spyOn(ofac, 'useGetIsSanctionedQuery').mockImplementation((arg) => {
if (arg === skipToken) {
return { data: undefined, refetch: jest.fn() }
}
return { data: true, refetch: jest.fn() }
})
const { result } = renderHook(() => useSanctionedAddress())
expect(result.current).toEqual(mockSafeAddress)
})

it('should skip sanction check if isRestricted is false', () => {
const mockSafeAddress = faker.finance.ethereumAddress()
const mockWalletAddress = faker.finance.ethereumAddress()
mockUseSafeAddress.mockReturnValue(mockSafeAddress)
mockUseWallet.mockReturnValue(connectedWalletBuilder().with({ address: mockWalletAddress }).build())

jest.spyOn(ofac, 'useGetIsSanctionedQuery').mockImplementation((arg) => {
if (arg === skipToken) {
return { data: undefined, refetch: jest.fn() }
}
return { data: true, refetch: jest.fn() }
})

const { result } = renderHook(() => useSanctionedAddress(false))
expect(result.current).toBeUndefined()
})
})
29 changes: 29 additions & 0 deletions src/hooks/useSanctionedAddress.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { useGetIsSanctionedQuery } from '@/store/ofac'
import useSafeAddress from './useSafeAddress'
import useWallet from './wallets/useWallet'
import { skipToken } from '@reduxjs/toolkit/query/react'

/**
* Checks if the opened Safe or the connected wallet are sanctioned and returns the sanctioned address.
* @param isRestricted the check is only performed if isRestricted is true.
* @returns address of sanctioned wallet or Safe
*/
export const useSanctionedAddress = (isRestricted = true) => {
const wallet = useWallet()
const safeAddress = useSafeAddress()

const { data: isWalletSanctioned } = useGetIsSanctionedQuery(isRestricted && wallet ? wallet.address : skipToken)

const { data: isSafeSanctioned } = useGetIsSanctionedQuery(
isRestricted && safeAddress !== '' ? safeAddress : skipToken,
)

if (isSafeSanctioned) {
return safeAddress
}
if (isWalletSanctioned) {
return wallet?.address
}
Comment on lines +21 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data contains the error message in case the useGetIsSanctionedQuery throws so I think it would falsely say that an address is sanctioned if the network request fails.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is not true. The error seems to be transformed and returned in the error field of useGetIsSanctionedQuery

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove the data assignment on error then inside the ofacApi query?

return { error: { status: 'CUSTOM_ERROR', data: (error as Error).message } }


return undefined
}
4 changes: 2 additions & 2 deletions src/store/ofac.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const ofacApi = createApi({
reducerPath: 'ofacApi',
baseQuery: noopBaseQuery,
endpoints: (builder) => ({
getIsSanctioned: builder.query({
getIsSanctioned: builder.query<boolean, string>({
async queryFn(address, { getState }) {
const state = getState()
const chain = selectChainById(state as RootState, chains.eth)
Expand All @@ -59,7 +59,7 @@ export const ofacApi = createApi({
const isAddressBlocked: boolean = await contract['isSanctioned'](address)
return { data: isAddressBlocked }
} catch (error) {
return { error: { status: 'CUSTOM_ERROR', data: (error as Error).message } }
return { error }
}
},
keepUnusedDataFor: 24 * 60 * 60, // 24 hours
Expand Down
Loading