-
Notifications
You must be signed in to change notification settings - Fork 24
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
Validate all Send Addresses against stellar.expert's list of malicious/unsafe addresses #245
Changes from 3 commits
2c19387
bdf23f9
e5612ba
0aa8238
99998dd
042e2c0
5df92e2
9c89d51
8d5e95f
92aa25d
b935f63
e948089
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ import { getNetworkConfig } from "helpers/getNetworkConfig"; | |
import { lumensFromStroops } from "helpers/stroopConversion"; | ||
import { logEvent } from "helpers/tracking"; | ||
import { useRedux } from "hooks/useRedux"; | ||
import { useFlaggedAccounts } from "hooks/useUnsafeAccounts"; | ||
import { | ||
ActionStatus, | ||
NetworkCongestion, | ||
|
@@ -147,6 +148,9 @@ export const CreateTransaction = ({ | |
initialFormData.isAccountFunded, | ||
); | ||
|
||
const [isAccountUnsafe, setIsAccountUnsafe] = useState(false); | ||
const [isAccountMalicious, setIsisAccountMalicious] = useState(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: there is an extra |
||
|
||
const knownAccount = | ||
knownAccounts[toAccountId] || knownAccounts[federationAddress || ""]; | ||
const [prevAddress, setPrevAddress] = useState( | ||
|
@@ -251,6 +255,19 @@ export const CreateTransaction = ({ | |
} | ||
}; | ||
|
||
const { flaggedAccounts } = useFlaggedAccounts(); | ||
|
||
const checkIfAccountIsFlagged = (accountId: string) => { | ||
const flaggedAccountData = flaggedAccounts.find( | ||
({ address }: { address: string }) => address === accountId, | ||
); | ||
if (flaggedAccountData?.tags) { | ||
const { tags } = flaggedAccountData; | ||
setIsAccountUnsafe(tags.includes("unsafe")); | ||
setIsisAccountMalicious(tags.includes("malicious")); | ||
} | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not resetting |
||
|
||
const checkAndSetIsAccountFunded = async (accountId: string) => { | ||
if (!accountId || !StrKey.isValidEd25519PublicKey(accountId)) { | ||
setIsAccountFunded(true); | ||
|
@@ -288,6 +305,9 @@ export const CreateTransaction = ({ | |
) { | ||
message = | ||
'Stellar address or public key is invalid. Public keys are uppercase and begin with letter "G."'; | ||
} else if (isAccountMalicious) { | ||
message = | ||
"This account has been flagged as being potentially malicious."; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if something is |
||
} | ||
|
||
errors[SendFormIds.SEND_TO] = message; | ||
|
@@ -445,6 +465,8 @@ export const CreateTransaction = ({ | |
|
||
setToAccountId(e.target.value); | ||
|
||
checkIfAccountIsFlagged(e.target.value); | ||
|
||
if (federationAddressFetchStatus) { | ||
setFederationAddressFetchStatus(null); | ||
} | ||
|
@@ -515,6 +537,14 @@ export const CreateTransaction = ({ | |
</RowEl> | ||
)} | ||
|
||
{isAccountUnsafe && ( | ||
piyalbasu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<RowEl> | ||
<InfoBlock variant={InfoBlockVariant.error}> | ||
<p>This account has been flagged as being potentially unsafe.</p> | ||
</InfoBlock> | ||
</RowEl> | ||
)} | ||
|
||
<RowEl> | ||
<CellEl> | ||
<Input | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
const UNSAFE_ACCOUNTS_URL = | ||
"https://api.stellar.expert/explorer/directory?limit=20000000&tag[]=malicious&tag[]=unsafe"; | ||
// setting limit very high as there doesn't appear to be a better way to get all entries from API | ||
|
||
const RESPONSE_TIMEOUT = 5000; | ||
// if API doesn't respond in this amount of time, we'll cancel the request | ||
|
||
export const getFlaggedAccounts = async () => { | ||
const controller = new AbortController(); | ||
const timeoutId = setTimeout(() => controller.abort(), RESPONSE_TIMEOUT); | ||
|
||
const flaggedAccountsRes = await fetch(UNSAFE_ACCOUNTS_URL, { | ||
signal: controller.signal, | ||
}); | ||
clearTimeout(timeoutId); | ||
const flaggedAccountsJson = await flaggedAccountsRes.json(); | ||
|
||
const { | ||
_embedded: { records: unsafeAccountsData }, | ||
} = flaggedAccountsJson; | ||
|
||
return unsafeAccountsData.map( | ||
({ address, tags }: { address: string; tags: [] }) => ({ address, tags }), | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// @ts-nocheck | ||
import { useEffect, useState } from "react"; | ||
import { FLAGGED_ACCOUNT_STORAGE_ID } from "constants/settings"; | ||
|
||
import { getFlaggedAccounts } from "helpers/getFlaggedAccounts"; | ||
|
||
interface FlaggedAccount { | ||
address: string; | ||
tags: [string]; | ||
} | ||
|
||
interface FlaggedAccounts extends Array<FlaggedAccount> {} | ||
|
||
export const useFlaggedAccounts = () => { | ||
const [flaggedAccounts, setFlaggedAccounts] = useState<FlaggedAccounts>([ | ||
{ address: "", tags: [] }, | ||
]); | ||
|
||
useEffect(() => { | ||
const fetchFlaggedAccounts = async () => { | ||
let accounts; | ||
|
||
try { | ||
accounts = await getFlaggedAccounts(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this function will throw an error if it doesn't resolve in time |
||
} catch (e) { | ||
accounts = | ||
JSON.parse(localStorage.getItem(FLAGGED_ACCOUNT_STORAGE_ID)) || []; | ||
} | ||
setFlaggedAccounts(accounts); | ||
localStorage.setItem( | ||
FLAGGED_ACCOUNT_STORAGE_ID, | ||
JSON.stringify(accounts), | ||
); | ||
}; | ||
fetchFlaggedAccounts(); | ||
}, []); | ||
|
||
return { flaggedAccounts }; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran into a very weird bug with
react-scripts
. It was falsely throwing an eslint error in a module exporting an array. What I've done here is kind of a band aid fix: it upgrades the versions of typescript-eslint thatreact-scripts
uses. Another option is to updatereact-scripts
itself BUT that causes a whole host of other eslint issues to crop up which were out of scope of this issue. This was the least invasive method I was able to find.Further reading here:
https://stackoverflow.com/a/63919395
facebook/create-react-app#9515
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen this one before. I think your fix is good. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah Haven't seen
resolutions
before but that seems like a useful tool for the toolbox!