Skip to content

Commit

Permalink
feat: Remove the Phishfort list from the PhishingController (#4621)
Browse files Browse the repository at this point in the history
## Explanation

This PR removes PhishFort from the core MetaMask repository due to the
termination of our contract with them for phishing detection. The
immediate goal is to eliminate the PhishFort list from the
PhishingController to prevent any potential false positives they might
introduce. Given the urgency, this PR focuses solely on removing
PhishFort from the client-side. Subsequent updates will address the
removal of PhishFort from the API and the related test cases.

## References

<!-- 
Link any related issues, discussions, or documents that provide
additional context for these changes.
-->

## Changelog

### `@metamask/phishing-controller`

- **REMOVED**: PhishFort list and related references have been removed
from the PhishingController.

## Checklist

- [x] I've updated the test suite to reflect the removal of PhishFort.
- [x] I've updated relevant documentation to ensure no references to
PhishFort remain.
- [x] No breaking changes have been introduced.
  • Loading branch information
AugmentedMode authored Aug 20, 2024
1 parent 462f95f commit 979b227
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 46 deletions.
18 changes: 0 additions & 18 deletions packages/phishing-controller/src/PhishingController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1060,15 +1060,6 @@ describe('PhishingController', () => {
name: ListNames.MetaMask,
version: 0,
},
{
allowlist: [],
blocklist: [],
fuzzylist: [],
tolerance: 0,
lastUpdated: 1,
name: ListNames.Phishfort,
version: 0,
},
]);
});

Expand Down Expand Up @@ -1127,15 +1118,6 @@ describe('PhishingController', () => {
lastUpdated: 2,
name: ListNames.MetaMask,
},
{
blocklist: [],
allowlist: [],
fuzzylist: [],
tolerance: 0,
version: 0,
lastUpdated: 1,
name: ListNames.Phishfort,
},
]);
});

Expand Down
26 changes: 3 additions & 23 deletions packages/phishing-controller/src/PhishingController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export type EthPhishingResponse = {
*
* type defining expected type of the stalelist.json file.
* @property eth_phishing_detect_config - Stale list sourced from eth-phishing-detect's config.json.
* @property phishfort_hotlist - Stale list sourced from phishfort's hotlist.json. Only includes blocklist. Deduplicated entries from eth_phishing_detect_config.
* @property tolerance - Fuzzy match tolerance level
* @property lastUpdated - Timestamp of last update.
* @property version - Stalelist data structure iteration.
Expand All @@ -59,9 +58,6 @@ export type PhishingStalelist = {
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/naming-convention
eth_phishing_detect_config: Record<ListTypes, string[]>;
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/naming-convention
phishfort_hotlist: Record<ListTypes, string[]>;
tolerance: number;
version: number;
lastUpdated: number;
Expand Down Expand Up @@ -145,7 +141,6 @@ export type Hotlist = HotlistDiff[];
* These are the keys denoting lists consumed by the upstream data provider.
*/
export enum ListKeys {
PhishfortHotlist = 'phishfort_hotlist',
EthPhishingDetectConfig = 'eth_phishing_detect_config',
}

Expand All @@ -154,15 +149,13 @@ export enum ListKeys {
*/
export enum ListNames {
MetaMask = 'MetaMask',
Phishfort = 'Phishfort',
}

/**
* Maps from downstream client attribution name
* to list key sourced from upstream data provider.
*/
const phishingListNameKeyMap = {
[ListNames.Phishfort]: ListKeys.PhishfortHotlist,
[ListNames.MetaMask]: ListKeys.EthPhishingDetectConfig,
};

Expand All @@ -172,7 +165,6 @@ const phishingListNameKeyMap = {
*/
export const phishingListKeyNameMap = {
[ListKeys.EthPhishingDetectConfig]: ListNames.MetaMask,
[ListKeys.PhishfortHotlist]: ListNames.Phishfort,
};

const controllerName = 'PhishingController';
Expand Down Expand Up @@ -497,35 +489,23 @@ export class PhishingController extends BaseController<

// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/naming-convention
const { phishfort_hotlist, eth_phishing_detect_config, ...partialState } =
const { eth_phishing_detect_config, ...partialState } =
stalelistResponse.data;

const phishfortListState: PhishingListState = {
...phishfort_hotlist,
...partialState,
fuzzylist: [], // Phishfort hotlist doesn't contain a fuzzylist
allowlist: [], // Phishfort hotlist doesn't contain an allowlist
name: phishingListKeyNameMap.phishfort_hotlist,
};
const metamaskListState: PhishingListState = {
...eth_phishing_detect_config,
...partialState,
name: phishingListKeyNameMap.eth_phishing_detect_config,
};
// Correctly shaping eth-phishing-detect state by applying hotlist diffs to the stalelist.
const newPhishfortListState: PhishingListState = applyDiffs(
phishfortListState,
hotlistDiffsResponse.data,
ListKeys.PhishfortHotlist,
);

const newMetaMaskListState: PhishingListState = applyDiffs(
metamaskListState,
hotlistDiffsResponse.data,
ListKeys.EthPhishingDetectConfig,
);

this.update((draftState) => {
draftState.phishingLists = [newMetaMaskListState, newPhishfortListState];
draftState.phishingLists = [newMetaMaskListState];
});
this.updatePhishingDetector();
}
Expand Down
10 changes: 5 additions & 5 deletions packages/phishing-controller/src/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,11 @@ describe('applyDiffs', () => {
const result = applyDiffs(
testExistingState,
[exampleAddDiff],
ListKeys.PhishfortHotlist,
ListKeys.EthPhishingDetectConfig,
);
expect(result).toStrictEqual({
...testExistingState,
name: ListNames.Phishfort,
name: ListNames.MetaMask,
});
});

Expand All @@ -137,14 +137,14 @@ describe('applyDiffs', () => {
const result = applyDiffs(
testExistingState,
[
{ ...exampleAddDiff, timestamp: 1674773009 },
{ ...exampleAddDiff, timestamp: 1674773005 },
{ ...exampleRemoveDiff, timestamp: 1674773004 },
],
ListKeys.PhishfortHotlist,
ListKeys.EthPhishingDetectConfig,
);
expect(result).toStrictEqual({
...testExistingState,
name: ListNames.Phishfort,
name: ListNames.MetaMask,
});
});
});
Expand Down

0 comments on commit 979b227

Please sign in to comment.