ButterflyEffect/uniswap-interface-uncensored
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: reduce severity of phishing filter to allow url token names (#6249)

Browse Source 
* feat: reduce severity of phishing filter to allow url token names

* fix # positions, prototype progressive risk check w/ list presence

* functional progressive risk check functionality

* better name for tokenList

* remove logs/todos

* make it work for mini-portfolio

* getUniqueAddressesFromPositions

* useTokenContractsConstant

* adds comment documenting the positionInfo->positionDetails type mapping

* safe->filtered positions in the Pool page

* use for loop instead of reduce

* WIP: token constants lookup table consolidating chain data and list data

* remove lookup table to pr#6268

* use NEVER_RELOAD

* use a counter on tokenList inclusion

* pr feedback - code simplification

* add tests

* tinaszheng pr feedback

* fix incorrect undefined fn signature

* forEach -> for loop

* add comment explainer to useFilterPossiblyMaliciousPositions

* simplify and comments re: filtration logic

* fix tests
This commit is contained in:
Jordan Frankfurt 2023-04-24 10:11:04 -05:00 committed by GitHub
parent ffdd311be9
commit bc48b4fb0e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 266 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
src/components/AccountDrawer/MiniPortfolio/Pools/index.tsx
View File

@ -7,13 +7,13 @@ import { useWeb3React } from '@web3-react/core'
import { useToggleAccountDrawer } from 'components/AccountDrawer'
import Row from 'components/Row'
import { MouseoverTooltip } from 'components/Tooltip'
import { useFilterPossiblyMaliciousPositions } from 'hooks/useFilterPossiblyMaliciousPositions'
import { EmptyWalletModule } from 'nft/components/profile/view/EmptyWalletContent'
import { useCallback, useMemo, useReducer } from 'react'
import { useNavigate } from 'react-router-dom'
import styled from 'styled-components/macro'
import { ThemedText } from 'theme'
import { switchChain } from 'utils/switchChain'
import { hasURL } from 'utils/urlChecks'
import { ExpandoRow } from '../ExpandoRow'
import { PortfolioLogo } from '../PortfolioLogo'
@ -22,24 +22,55 @@ import { PositionInfo } from './cache'
import { useFeeValues } from './hooks'
import useMultiChainPositions from './useMultiChainPositions'
/*
This hook takes an array of PositionInfo objects (format used by the Uniswap Labs gql API).
The hook access PositionInfo.details (format used by the NFT position contract),
filters the PositionDetails data for malicious content,
and then returns the original data in its original format.
*/
function useFilterPossiblyMaliciousPositionInfo(positions: PositionInfo[] | undefined): PositionInfo[] {
const tokenIdsToPositionInfo: Record<string, PositionInfo> = useMemo(
() =>
positions
? positions.reduce((acc, position) => ({ ...acc, [position.details.tokenId.toString()]: position }), {})
: {},
[positions]
)
const positionDetails = useMemo(() => positions?.map((position) => position.details) ?? [], [positions])
const filteredPositionDetails = useFilterPossiblyMaliciousPositions(positionDetails)
return useMemo(
() => filteredPositionDetails.map((positionDetails) => tokenIdsToPositionInfo[positionDetails.tokenId.toString()]),
[filteredPositionDetails, tokenIdsToPositionInfo]
)
}
export default function Pools({ account }: { account: string }) {
const { positions, loading } = useMultiChainPositions(account)
const filteredPositions = useFilterPossiblyMaliciousPositionInfo(positions)
const [showClosed, toggleShowClosed] = useReducer((showClosed) => !showClosed, false)
const [openPositions, closedPositions] = useMemo(() => {
const openPositions: PositionInfo[] = []
const closedPositions: PositionInfo[] = []
positions?.forEach((position) => (position.closed ? closedPositions : openPositions).push(position))
for (let i = 0; i < filteredPositions.length; i++) {
const position = filteredPositions[i]
if (position.closed) {
closedPositions.push(position)
} else {
openPositions.push(position)
}
}
return [openPositions, closedPositions]
}, [positions])
}, [filteredPositions])
const toggleWalletDrawer = useToggleAccountDrawer()
if (!positions || loading) {
if (!filteredPositions || loading) {
return <PortfolioSkeleton />
}
if (positions?.length === 0) {
if (filteredPositions.length === 0) {
return <EmptyWalletModule type="pool" onNavigateClick={toggleWalletDrawer} />
}
@ -111,12 +142,6 @@ function PositionListItem({ positionInfo }: { positionInfo: PositionInfo }) {
[chainId, pool.token0.address, pool.token0.symbol, pool.token1.address, pool.token1.symbol]
)
const shouldHidePosition = hasURL(pool.token0.symbol) || hasURL(pool.token1.symbol)
if (shouldHidePosition) {
return null
}
return (
<TraceEvent
events={[BrowserEvent.onClick]}

65
src/components/PositionListItem/PositionListItem.test.tsx
View File

@ -3,22 +3,20 @@ import { SupportedChainId, Token, WETH9 } from '@uniswap/sdk-core'
import { FeeAmount, Pool } from '@uniswap/v3-sdk'
import { USDC_MAINNET } from 'constants/tokens'
import { useToken } from 'hooks/Tokens'
import { usePool } from 'hooks/usePools'
import { PoolState } from 'hooks/usePools'
import { usePool } from 'hooks/usePools'
import { mocked } from 'test-utils/mocked'
import { render } from 'test-utils/render'
import { unwrappedToken } from 'utils/unwrappedToken'
import PositionListItem from '.'
jest.mock('utils/unwrappedToken')
jest.mock('hooks/usePools')
jest.mock('utils/unwrappedToken')
jest.mock('hooks/Tokens')
// eslint-disable-next-line react/display-name
jest.mock('components/DoubleLogo', () => () => <div />)
jest.mock('components/DoubleLogo')
jest.mock('@web3-react/core', () => {
const web3React = jest.requireActual('@web3-react/core')
@ -30,59 +28,34 @@ jest.mock('@web3-react/core', () => {
}
})
const susToken0Address = '0x39AA39c021dfbaE8faC545936693aC917d5E7563'
beforeEach(() => {
const susToken0 = new Token(1, susToken0Address, 8, 'https://www.example.com', 'example.com coin')
mocked(useToken).mockImplementation((tokenAddress?: string | null | undefined) => {
if (!tokenAddress) return null
if (tokenAddress === susToken0.address) return susToken0
return new Token(1, tokenAddress, 8, 'symbol', 'name')
return new Token(1, tokenAddress, 6, 'symbol', 'name')
})
mocked(usePool).mockReturnValue([
PoolState.EXISTS,
new Pool(susToken0, USDC_MAINNET, FeeAmount.HIGH, '2437312313659959819381354528', '10272714736694327408', -69633),
// tokenA: Token, tokenB: Token, fee: FeeAmount, sqrtRatioX96: BigintIsh, liquidity: BigintIsh, tickCurrent: number
new Pool(
USDC_MAINNET,
WETH9[SupportedChainId.MAINNET],
FeeAmount.MEDIUM,
'1745948049099224684665158875285708',
'4203610460178577802',
200019
),
])
mocked(unwrappedToken).mockReturnValue(susToken0)
})
test('PositionListItem should not render when token0 symbol contains a url', () => {
const positionDetails = {
token0: susToken0Address,
token1: USDC_MAINNET.address,
tokenId: BigNumber.from(436148),
fee: 100,
liquidity: BigNumber.from('0x5c985aff8059be04'),
tickLower: -800,
tickUpper: 1600,
}
const { container } = render(<PositionListItem {...positionDetails} />)
expect(container).toBeEmptyDOMElement()
})
test('PositionListItem should not render when token1 symbol contains a url', () => {
const positionDetails = {
token0: USDC_MAINNET.address,
token1: susToken0Address,
tokenId: BigNumber.from(436148),
fee: 100,
liquidity: BigNumber.from('0x5c985aff8059be04'),
tickLower: -800,
tickUpper: 1600,
}
const { container } = render(<PositionListItem {...positionDetails} />)
expect(container).toBeEmptyDOMElement()
})
test('PositionListItem should render a position', () => {
const positionDetails = {
token0: USDC_MAINNET.address,
token1: WETH9[SupportedChainId.MAINNET].address,
tokenId: BigNumber.from(436148),
fee: 100,
liquidity: BigNumber.from('0x5c985aff8059be04'),
tickLower: -800,
tickUpper: 1600,
tokenId: BigNumber.from(479689),
fee: FeeAmount.MEDIUM,
liquidity: BigNumber.from('1341008833950736'),
tickLower: 200040,
tickUpper: 202560,
}
const { container } = render(<PositionListItem {...positionDetails} />)
expect(container).not.toBeEmptyDOMElement()

7
src/components/PositionListItem/index.tsx
View File

@ -17,7 +17,6 @@ import styled from 'styled-components/macro'
import { HideSmall, MEDIA_WIDTHS, SmallOnly, ThemedText } from 'theme'
import { formatTickPrice } from 'utils/formatTickPrice'
import { unwrappedToken } from 'utils/unwrappedToken'
import { hasURL } from 'utils/urlChecks'
import { DAI, USDC_MAINNET, USDT, WBTC, WRAPPED_NATIVE_CURRENCY } from '../../constants/tokens'
@ -203,12 +202,6 @@ export default function PositionListItem({
const removed = liquidity?.eq(0)
const shouldHidePosition = hasURL(token0?.symbol) || hasURL(token1?.symbol)
if (shouldHidePosition) {
return null
}
return (
<LinkRow to={positionSummaryLink}>
<RowBetween>

126
src/hooks/useFilterPossiblyMaliciousPositions.test.ts Normal file
View File

@ -0,0 +1,126 @@
import { BigNumber } from '@ethersproject/bignumber'
import { CallState } from '@uniswap/redux-multicall'
import { renderHook } from 'test-utils/render'
import { PositionDetails } from 'types/position'
import { useFilterPossiblyMaliciousPositions } from './useFilterPossiblyMaliciousPositions'
import { useTokenContractsConstant } from './useTokenContractsConstant'
jest.mock('./useTokenContractsConstant')
jest.mock('./Tokens', () => {
return {
useDefaultActiveTokens: () => ({
'0x4200000000000000000000000000000000000006': {
chainId: 10,
address: '0x4200000000000000000000000000000000000006',
name: 'Wrapped Ether',
symbol: 'WETH',
decimals: 18,
logoURI: 'https://ethereum-optimism.github.io/data/WETH/logo.png',
extensions: {},
},
'0xDA10009cBd5D07dd0CeCc66161FC93D7c9000da1': {
chainId: 10,
address: '0xDA10009cBd5D07dd0CeCc66161FC93D7c9000da1',
name: 'Dai Stablecoin',
symbol: 'DAI',
decimals: 18,
logoURI: 'https://ethereum-optimism.github.io/data/DAI/logo.svg',
extensions: {
optimismBridgeAddress: '0x467194771dAe2967Aef3ECbEDD3Bf9a310C76C65',
},
},
}),
}
})
const mockUseTokenContractsConstant = useTokenContractsConstant as jest.MockedFunction<typeof useTokenContractsConstant>
beforeEach(() => {
mockUseTokenContractsConstant.mockReturnValue([])
})
const positions: PositionDetails[] = [
{
tokenId: BigNumber.from('0x02'),
fee: 3000,
feeGrowthInside0LastX128: BigNumber.from('0x168af578d503c230c7fabeef7c38'),
feeGrowthInside1LastX128: BigNumber.from('0x9691f41769e1a6a196e8f5bcddf89c'),
liquidity: BigNumber.from('0xa0deffe49ff1158e'),
nonce: BigNumber.from('0x00'),
operator: '0x0000000000000000000000000000000000000000',
tickLower: -887220,
tickUpper: 887220,
token0: '0x4200000000000000000000000000000000000006',
token1: '0xDA10009cBd5D07dd0CeCc66161FC93D7c9000da1',
tokensOwed0: BigNumber.from('0x00'),
tokensOwed1: BigNumber.from('0x00'),
},
{
tokenId: BigNumber.from('0x03'),
fee: 3000,
feeGrowthInside0LastX128: BigNumber.from('0x00'),
feeGrowthInside1LastX128: BigNumber.from('0x00'),
liquidity: BigNumber.from('0x0e422f1864e669076d'),
nonce: BigNumber.from('0x00'),
operator: '0x0000000000000000000000000000000000000000',
tickLower: 72660,
tickUpper: 80820,
token0: '0x4200000000000000000000000000000000000006',
token1: '0xDA10009cBd5D07dd0CeCc66161FC93D7c9000da1',
tokensOwed0: BigNumber.from('0x00'),
tokensOwed1: BigNumber.from('0x00'),
},
{
tokenId: BigNumber.from('0x047aa5'),
fee: 3000,
feeGrowthInside0LastX128: BigNumber.from('0x00'),
feeGrowthInside1LastX128: BigNumber.from('0x00'),
liquidity: BigNumber.from('0x8ac7230489e80001'),
nonce: BigNumber.from('0x00'),
operator: '0x0000000000000000000000000000000000000000',
tickLower: -120,
tickUpper: 120,
token0: '0x75E5509029c85fE08e4934B1275c5575aA5538bE',
token1: '0xB85C51F89788C1B3Bd4568f773e8FfB40cA587Bb',
tokensOwed0: BigNumber.from('0x00'),
tokensOwed1: BigNumber.from('0x00'),
},
]
const unsafeReturnValue: CallState[] = [
{
valid: true,
loading: false,
syncing: false,
result: ['Uniswap-LP.org'],
error: false,
},
{
valid: true,
loading: false,
syncing: false,
result: ['Claim Rewards'],
error: false,
},
]
describe('useFilterPossiblyMaliciousPositions', () => {
it('filters out unsafe positions', async () => {
mockUseTokenContractsConstant.mockReturnValue(unsafeReturnValue)
const { result } = renderHook(() => useFilterPossiblyMaliciousPositions(positions))
expect(result.current.some((position) => position.token1 === '0xB85C51F89788C1B3Bd4568f773e8FfB40cA587Bb')).toEqual(
false
)
})
it('checks the chain for addresses not on the token list', async () => {
mockUseTokenContractsConstant.mockReturnValue(unsafeReturnValue)
renderHook(() => useFilterPossiblyMaliciousPositions(positions))
expect(mockUseTokenContractsConstant).toHaveBeenCalledWith(
['0x75E5509029c85fE08e4934B1275c5575aA5538bE', '0xB85C51F89788C1B3Bd4568f773e8FfB40cA587Bb'],
'symbol'
)
})
})

71
src/hooks/useFilterPossiblyMaliciousPositions.ts Normal file
View File

@ -0,0 +1,71 @@
import { Token } from '@uniswap/sdk-core'
import { useMemo } from 'react'
import { PositionDetails } from 'types/position'
import { hasURL } from 'utils/urlChecks'
import { useDefaultActiveTokens } from './Tokens'
import { useTokenContractsConstant } from './useTokenContractsConstant'
function getUniqueAddressesFromPositions(positions: PositionDetails[]): string[] {
return Array.from(
new Set(positions.reduce<string[]>((acc, position) => acc.concat(position.token0, position.token1), []))
)
}
/*
* This function is an attempt to filter out an observed phishing attack from LP list UIs.
* Attackers would airdrop valueless LP positions with urls in the symbol to render phishing sites into users' LP position list view.
*
* Our approach to filtering these out without naively prohibiting all valid URL symbols is to:
* 1. allow any pair if both tokens are in the supported list
* 2. allow one url if one token is in the supported list
* 3. allow no urls if neither token is in the supported list
*
* The hope is that this approach removes the cheapest version of the attack without punishing non-malicious url symbols
*/
export function useFilterPossiblyMaliciousPositions(positions: PositionDetails[]): PositionDetails[] {
const activeTokensList = useDefaultActiveTokens()
const nonListPositionTokenAddresses = useMemo(
() => getUniqueAddressesFromPositions(positions).filter((address) => !activeTokensList[address]),
[positions, activeTokensList]
)
const symbolCallStates = useTokenContractsConstant(nonListPositionTokenAddresses, 'symbol')
const addressesToSymbol: Record<string, string> = useMemo(() => {
const result: Record<string, string> = {}
for (let i = 0; i < nonListPositionTokenAddresses.length; i++) {
const callResult = symbolCallStates[i].result
if (!callResult) continue
const address = nonListPositionTokenAddresses[i]
result[address] = callResult as unknown as string
}
return result
}, [nonListPositionTokenAddresses, symbolCallStates])
return useMemo(
() =>
positions.filter((position) => {
let tokensInListCount = 0
const token0FromList = activeTokensList[position.token0] as Token | undefined
const token1FromList = activeTokensList[position.token1] as Token | undefined
if (token0FromList) tokensInListCount++
if (token1FromList) tokensInListCount++
// if both tokens are in the list, then we let both have url symbols (so we don't check)
if (tokensInListCount === 2) return true
// check the token symbols to see if they contain a url
// prioritize the token entity from the list if it exists
// if the token isn't in the list, then use the data returned from chain calls
let urlSymbolCount = 0
if (hasURL(token0FromList?.symbol ?? addressesToSymbol[position.token0])) urlSymbolCount++
if (hasURL(token1FromList?.symbol ?? addressesToSymbol[position.token1])) urlSymbolCount++
// if one token is in the list, then one token can have a url symbol
if (tokensInListCount === 1 && urlSymbolCount < 2) return true
// if neither token is in the list, then neither can have a url symbol
return urlSymbolCount === 0
}),
[addressesToSymbol, positions, activeTokensList]
)
}

10
src/hooks/useTokenContractsConstant.ts Normal file
View File

@ -0,0 +1,10 @@
import { Interface } from '@ethersproject/abi'
import ERC20ABI from 'abis/erc20.json'
import { Erc20Interface } from 'abis/types/Erc20'
import { NEVER_RELOAD, useMultipleContractSingleData } from 'lib/hooks/multicall'
const ERC20Interface = new Interface(ERC20ABI) as Erc20Interface
export function useTokenContractsConstant(tokens: string[], field: 'name' | 'symbol' | 'decimals' | 'totalSupply') {
return useMultipleContractSingleData(tokens, ERC20Interface, field, undefined, NEVER_RELOAD)
}

5
src/pages/Pool/index.tsx
View File

@ -10,6 +10,7 @@ import PositionList from 'components/PositionList'
import { RowBetween, RowFixed } from 'components/Row'
import { SwitchLocaleLink } from 'components/SwitchLocaleLink'
import { isSupportedChain } from 'constants/chains'
import { useFilterPossiblyMaliciousPositions } from 'hooks/useFilterPossiblyMaliciousPositions'
import { useV3Positions } from 'hooks/useV3Positions'
import { useMemo } from 'react'
import { AlertTriangle, BookOpen, ChevronDown, ChevronsRight, Inbox, Layers } from 'react-feather'
@ -211,11 +212,13 @@ export default function Pool() {
[[], []]
) ?? [[], []]
const filteredPositions = useMemo(
const userSelectedPositionSet = useMemo(
() => [...openPositions, ...(userHideClosedPositions ? [] : closedPositions)],
[closedPositions, openPositions, userHideClosedPositions]
)
const filteredPositions = useFilterPossiblyMaliciousPositions(userSelectedPositionSet)
if (!isSupportedChain(chainId)) {
return <WrongNetworkCard />
}