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.

Sign up for GitHub

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

sturdier check #5866

Merged
merged 12 commits into from
May 5, 2023
+138 −31
Merged

sturdier check #5866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
522ca63
sturdier check
witmicko Mar 1, 2023
ab3be58
apply feedback
witmicko Mar 1, 2023
f0bbb02
apply feedback
witmicko Mar 1, 2023
12a0a3b
unit test
witmicko Mar 1, 2023
c0ea7b6
check phishing and deeplinks
witmicko Mar 8, 2023
4b8ddc9
feedback
witmicko Mar 29, 2023
d4a2671
resolve
witmicko Apr 20, 2023
0a30d24
url check
witmicko Apr 12, 2023
5187097
more tests
witmicko Apr 13, 2023
73a5277
Merge branch 'main' into nft-view-protect-2
witmicko May 2, 2023
58dd224
Merge branch 'main' into nft-view-protect-2
witmicko May 4, 2023
3ea7ec1
Merge branch 'main' into nft-view-protect-2
cortisiko May 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 44 additions & 31 deletions app/components/UI/CollectibleOverview/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
SafeAreaView,
TouchableWithoutFeedback,
} from 'react-native';

import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { baseStyles } from '../../../styles/common';
Expand All @@ -28,6 +29,7 @@ import { toLocaleDate } from '../../../util/date';
import { renderFromWei } from '../../../util/number';
import { renderShortAddress } from '../../../util/address';
import { isMainNet } from '../../../util/networks';
import { isLinkSafe } from '../../../util/linkCheck';
import etherscanLink from '@metamask/etherscan-link';
import {
addFavoriteCollectible,
Expand Down Expand Up @@ -131,6 +133,10 @@ const createStyles = (colors) =>
},
});

const FieldType = {
Link: 'Link',
Text: 'Text',
};
/**
* View that displays the information of a specific ERC-721 Token
*/
Expand Down Expand Up @@ -166,11 +172,10 @@ const CollectibleOverview = ({
}, [collectible.description]);

const renderCollectibleInfoRow = useCallback(
(key, value, onPress) => {
({ key, value, onPress, type }) => {
if (!value) return null;

if (value.toLowerCase().includes('javascript')) {
return null;
if (type === FieldType.Link) {
if (!isLinkSafe(value)) return null;
}
return (
<View style={styles.collectibleInfoContainer} key={key}>
Expand Down Expand Up @@ -203,42 +208,50 @@ const CollectibleOverview = ({
);

const renderCollectibleInfo = () => [
renderCollectibleInfoRow(
strings('collectible.collectible_token_standard'),
collectible?.standard,
),
renderCollectibleInfoRow(
strings('collectible.collectible_last_sold'),
collectible?.lastSale?.event_timestamp &&
renderCollectibleInfoRow({
key: strings('collectible.collectible_token_standard'),
value: collectible?.standard,
type: FieldType.Text,
}),
renderCollectibleInfoRow({
key: strings('collectible.collectible_last_sold'),
value:
collectible?.lastSale?.event_timestamp &&
toLocaleDate(
new Date(collectible?.lastSale?.event_timestamp),
).toString(),
),
renderCollectibleInfoRow(
strings('collectible.collectible_last_price_sold'),
collectible?.lastSale?.total_price &&
type: FieldType.Text,
}),
renderCollectibleInfoRow({
key: strings('collectible.collectible_last_price_sold'),
value:
collectible?.lastSale?.total_price &&
`${renderFromWei(collectible?.lastSale?.total_price)} ETH`,
),
renderCollectibleInfoRow(
strings('collectible.collectible_source'),
collectible?.imageOriginal,
() => openLink(collectible?.imageOriginal),
),
renderCollectibleInfoRow(
strings('collectible.collectible_link'),
collectible?.externalLink,
() => openLink(collectible?.externalLink),
),
renderCollectibleInfoRow(
strings('collectible.collectible_asset_contract'),
renderShortAddress(collectible?.address),
() => {
type: FieldType.Text,
}),
renderCollectibleInfoRow({
key: strings('collectible.collectible_source'),
value: collectible?.imageOriginal,
onPress: () => openLink(collectible?.imageOriginal),
type: FieldType.Link,
}),
renderCollectibleInfoRow({
key: strings('collectible.collectible_link'),
value: collectible?.externalLink,
onPress: () => openLink(collectible?.externalLink),
type: FieldType.Link,
}),
renderCollectibleInfoRow({
key: strings('collectible.collectible_asset_contract'),
value: renderShortAddress(collectible?.address),
onPress: () => {
if (isMainNet(chainId))
openLink(
etherscanLink.createTokenTrackerLink(collectible?.address, chainId),
);
},
),
type: FieldType.Text,
}),
];

const collectibleToFavorites = useCallback(() => {
Expand Down
47 changes: 47 additions & 0 deletions app/util/linkCheck.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/* eslint-disable no-script-url */
import isLinkSafe from './linkCheck';

jest.mock('../core/Engine', () => ({
context: {
PhishingController: {
maybeUpdateState: jest.fn(),
test: jest.fn((url: string) => {
if (url === 'phishing.com') return { result: true };
return { result: false };
}),
},
},
}));

describe('linkCheck', () => {
it('should correctly check links for safety', () => {
expect(isLinkSafe('htps://ww.example.com/')).toEqual(false);
expect(isLinkSafe('https://ww.example.com/')).toEqual(true);
expect(isLinkSafe('http://example com/page?id=123')).toEqual(false);
expect(isLinkSafe('https://www.example.com/')).toEqual(true);
expect(isLinkSafe('http://phishing.com')).toEqual(false);
expect(
isLinkSafe(
'https://metamask.app.link/send/pay-Contract-Address@chain-id/transfer?address=Receiver-Address&uint256=1e21',
sethkfman marked this conversation as resolved.
Show resolved Hide resolved
),
).toEqual(false);

expect(isLinkSafe('javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('j&Tab;avascript:alert(1);')).toEqual(false);
expect(isLinkSafe('&Tab;javascript:alert(1);&tab;')).toEqual(false);
expect(isLinkSafe('javas\x00cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x07cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x0Dcript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x0Acript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x08cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x02cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x03cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x04cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x01cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x05cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x0Bcript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x09cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x06cript:javascript:alert(1)')).toEqual(false);
expect(isLinkSafe('javas\x0Ccript:javascript:alert(1)')).toEqual(false);
});
});
35 changes: 35 additions & 0 deletions app/util/linkCheck.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import Url from 'url-parse';
import isUrl from 'is-url';
import { PhishingController as PhishingControllerClass } from '@metamask/phishing-controller';
import Engine from '../core/Engine';
const ALLOWED_PROTOCOLS = ['http:', 'https:'];
const DENYLISTED_DOMAINS = ['metamask.app.link'];

sethkfman marked this conversation as resolved.
Show resolved Hide resolved
const isAllowedProtocol = (protocol: string): boolean =>
ALLOWED_PROTOCOLS.includes(protocol);

const isAllowedHostname = (hostname: string): boolean => {
const { PhishingController } = Engine.context as {
PhishingController: PhishingControllerClass;
};
PhishingController.maybeUpdateState();
const phishingControllerTestResult = PhishingController.test(hostname);
witmicko marked this conversation as resolved.
Show resolved Hide resolved

return !(
phishingControllerTestResult.result || DENYLISTED_DOMAINS.includes(hostname)
);
};

export const isLinkSafe = (link: string): boolean => {
try {
const url = new Url(link);
const { protocol, hostname, href } = url;
return (
isUrl(href) && isAllowedProtocol(protocol) && isAllowedHostname(hostname)
);
} catch (err) {
return false;
}
};

export default isLinkSafe;
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@
"@storybook/react-native": "^5.3.25",
"@testing-library/react-hooks": "^8.0.1",
"@types/enzyme": "^3.10.9",
"@types/is-url": "^1.2.30",
"@types/jest": "^27.0.1",
"@types/react": "^17.0.11",
"@types/react-native": "^0.64.10",
Expand All @@ -347,6 +348,7 @@
"@types/react-native-vector-icons": "^6.4.8",
"@types/react-native-video": "^5.0.13",
"@types/redux-mock-store": "^1.0.3",
"@types/url-parse": "^1.4.8",
"@typescript-eslint/eslint-plugin": "^4.20.0",
"@typescript-eslint/parser": "^4.20.0",
"@wdio/appium-service": "^7.19.1",
Expand Down
10 changes: 10 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5574,6 +5574,11 @@
resolved "https://registry.yarnpkg.com/@types/is-glob/-/is-glob-4.0.2.tgz#c243dd0d09eac2992130142419ff2308ffd988bf"
integrity sha512-4j5G9Y5jljDSICQ1R2f/Rcyoj6DZmYGneny+p/cDkjep0rkqNg0W73Ty0bVjMUTZgLXHf8oiMjg1XC3CDwCz+g==

"@types/is-url@^1.2.30":
version "1.2.30"
resolved "https://registry.yarnpkg.com/@types/is-url/-/is-url-1.2.30.tgz#85567e8bee4fee69202bc3448f9fb34b0d56c50a"
integrity sha512-AnlNFwjzC8XLda5VjRl4ItSd8qp8pSNowvsut0WwQyBWHpOxjxRJm8iO6uETWqEyLdYdb9/1j+Qd9gQ4l5I4fw==

"@types/istanbul-lib-coverage@*", "@types/istanbul-lib-coverage@^2.0.0", "@types/istanbul-lib-coverage@^2.0.1":
version "2.0.3"
resolved "https://registry.yarnpkg.com/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.3.tgz#4ba8ddb720221f432e443bd5f9117fd22cfd4762"
Expand Down Expand Up @@ -5952,6 +5957,11 @@
resolved "https://registry.yarnpkg.com/@types/ua-parser-js/-/ua-parser-js-0.7.36.tgz#9bd0b47f26b5a3151be21ba4ce9f5fa457c5f190"
integrity sha512-N1rW+njavs70y2cApeIw1vLMYXRwfBy+7trgavGuuTfOd7j1Yh7QTRc/yqsPl6ncokt72ZXuxEU0PiCp9bSwNQ==

"@types/url-parse@^1.4.8":
version "1.4.8"
resolved "https://registry.yarnpkg.com/@types/url-parse/-/url-parse-1.4.8.tgz#c3825047efbca1295b7f1646f38203d9145130d6"
integrity sha512-zqqcGKyNWgTLFBxmaexGUKQyWqeG7HjXj20EuQJSJWwXe54BjX0ihIo5cJB9yAQzH8dNugJ9GvkBYMjPXs/PJw==

"@types/[email protected]", "@types/uuid@^8.3.0":
version "8.3.1"
resolved "https://registry.yarnpkg.com/@types/uuid/-/uuid-8.3.1.tgz#1a32969cf8f0364b3d8c8af9cc3555b7805df14f"
Expand Down