fix(AssetsController): remove ws metadata population

packages/assets-controller/CHANGELOG.md

@@ -14,6 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Bump `@metamask/keyring-controller` from `^27.1.0` to `^27.1.1` ([#9791](https://github.com/MetaMask/core/pull/9791))
 - Bump `@metamask/network-enablement-controller` from `^6.0.2` to `^6.0.3` ([#9791](https://github.com/MetaMask/core/pull/9791))
 
+### Fixed
+
+- Stop publishing `assetsInfo` from websocket balance updates; metadata is now resolved from the Token API by `TokenDataSource` to prevent WS poisoning (incorrect WS symbols and no detection metadata). Prevents bypassing token detection spam filtering ([#9790](https://github.com/MetaMask/core/pull/9790))
+
 ## [13.1.1]
 
 ### Changed

packages/assets-controller/src/AssetsController.ts

@@ -896,8 +896,6 @@ export class AssetsController extends BaseController<
     this.#accountActivityDataSource = new AccountActivityDataSource({
       messenger: this.messenger,
       onActiveChainsUpdated: this.#onActiveChainsUpdated,
-      getAssetType: (assetId: Caip19AssetId): 'native' | 'erc20' | 'spl' =>
-        this.#getAssetType(assetId),
       onAssetsUpdate: (response, request): Promise<void> =>
         this.handleAssetsUpdate(
           response,

packages/assets-controller/src/data-sources/AccountActivityDataSource.test.ts

@@ -89,9 +89,8 @@ function createBalanceUpdate(overrides?: {
 }
 
 type SetupOptions = {
-  groupAccounts?: InternalAccount[];
+  groupAccounts?: InternalAccount[] | (() => InternalAccount[]);
   selectedAccount?: InternalAccount | null;
-  getAssetType?: (assetId: Caip19AssetId) => 'native' | 'erc20' | 'spl';
   onAssetsUpdate?: jest.Mock;
   onActiveChainsUpdated?: jest.Mock;
   state?: { activeChains?: ChainId[] };
@@ -102,7 +101,6 @@ type SetupResult = {
   rootMessenger: RootMessenger;
   onAssetsUpdate: jest.Mock;
   onActiveChainsUpdated: jest.Mock;
-  getAssetType: jest.Mock;
   triggerBalanceUpdated: (payload: {
     address: string;
     chain: string;
@@ -154,21 +152,17 @@ function setup(options: SetupOptions = {}): SetupResult {
 
   rootMessenger.registerActionHandler(
     'AccountTreeController:getAccountsFromSelectedAccountGroup',
-    () => groupAccounts,
+    () =>
+      typeof groupAccounts === 'function' ? groupAccounts() : groupAccounts,
   );
   rootMessenger.registerActionHandler(
     'AccountsController:getSelectedAccount',
     () => selectedAccount as InternalAccount,
   );
 
-  const getAssetType = jest
-    .fn()
-    .mockImplementation(options.getAssetType ?? ((): 'native' => 'native'));
-
   const dataSource = new AccountActivityDataSource({
     messenger: assetsControllerMessenger,
     onActiveChainsUpdated,
-    getAssetType,
     onAssetsUpdate,
     state,
   });
@@ -199,7 +193,6 @@ function setup(options: SetupOptions = {}): SetupResult {
     rootMessenger,
     onAssetsUpdate,
     onActiveChainsUpdated,
-    getAssetType,
     triggerBalanceUpdated,
     triggerStatusChanged,
     cleanup,
@@ -291,14 +284,6 @@ describe('AccountActivityDataSource', () => {
             [ETH_ASSET]: { amount: '1' },
           },
         },
-        assetsInfo: {
-          [ETH_ASSET]: {
-            type: 'native',
-            symbol: 'ETH',
-            name: 'ETH',
-            decimals: 18,
-          },
-        },
       });
       expect(request).toStrictEqual({
         accountsWithSupportedChains: [
@@ -311,7 +296,7 @@ describe('AccountActivityDataSource', () => {
       cleanup();
     });
 
-    it('converts a hex postBalance to a human-readable amount', async () => {
+    it('does not publish asset metadata from the websocket payload', async () => {
       const account = createMockAccount();
       const { onAssetsUpdate, triggerBalanceUpdated, cleanup } = setup({
         groupAccounts: [account],
@@ -321,35 +306,45 @@ describe('AccountActivityDataSource', () => {
         address: EVM_ADDRESS,
         chain: CHAIN_MAINNET,
         updates: [
-          createBalanceUpdate({ postBalance: { amount: '0x10aa6d94e80' } }),
+          createBalanceUpdate({
+            // On-chain symbol/name can be attacker-controlled (e.g. scam
+            // URLs), so it must never reach state; the Token API is the
+            // metadata source of truth.
+            asset: { unit: 'www.scam-url.example' },
+          }),
         ],
       });
 
       await Promise.resolve();
 
       expect(onAssetsUpdate).toHaveBeenCalledTimes(1);
       const [response] = onAssetsUpdate.mock.calls[0];
-      expect(response.assetsBalance[account.id][ETH_ASSET]).toStrictEqual({
-        amount: '0.00000114526056',
-      });
+      expect(response.assetsInfo).toBeUndefined();
 
       cleanup();
     });
 
-    it('resolves the asset type via the injected getAssetType', async () => {
-      const { getAssetType, triggerBalanceUpdated, cleanup } = setup({
-        getAssetType: () => 'erc20',
+    it('converts a hex postBalance to a human-readable amount', async () => {
+      const account = createMockAccount();
+      const { onAssetsUpdate, triggerBalanceUpdated, cleanup } = setup({
+        groupAccounts: [account],
       });
 
       triggerBalanceUpdated({
         address: EVM_ADDRESS,
         chain: CHAIN_MAINNET,
-        updates: [createBalanceUpdate()],
+        updates: [
+          createBalanceUpdate({ postBalance: { amount: '0x10aa6d94e80' } }),
+        ],
       });
 
       await Promise.resolve();
 
-      expect(getAssetType).toHaveBeenCalledWith(ETH_ASSET);
+      expect(onAssetsUpdate).toHaveBeenCalledTimes(1);
+      const [response] = onAssetsUpdate.mock.calls[0];
+      expect(response.assetsBalance[account.id][ETH_ASSET]).toStrictEqual({
+        amount: '0.00000114526056',
+      });
 
       cleanup();
     });
@@ -546,7 +541,7 @@ describe('AccountActivityDataSource', () => {
 
     it('swallows synchronous errors thrown while handling the event', async () => {
       const { onAssetsUpdate, triggerBalanceUpdated, cleanup } = setup({
-        getAssetType: () => {
+        groupAccounts: () => {
           throw new Error('boom');
         },
       });
@@ -771,7 +766,6 @@ describe('AccountActivityDataSource', () => {
       const dataSource = createAccountActivityDataSource({
         messenger: assetsControllerMessenger,
         onActiveChainsUpdated: jest.fn(),
-        getAssetType: () => 'native',
         onAssetsUpdate: jest.fn(),
       });
 

packages/assets-controller/src/data-sources/AccountActivityDataSource.ts

@@ -11,7 +11,6 @@ import type { AssetsControllerMessenger } from '../AssetsController.js';
 import { projectLogger, createModuleLogger } from '../logger.js';
 import type {
   AssetBalance,
-  AssetMetadata,
   ChainId,
   Caip19AssetId,
   DataRequest,
@@ -36,15 +35,23 @@ const log = createModuleLogger(projectLogger, CONTROLLER_NAME);
  * Convert AccountActivityMessage balance updates into a {@link DataResponse}
  * for AssetsController.
  *
+ * Only balances are published. The websocket payload's asset `unit` echoes the
+ * on-chain contract symbol, which is attacker-controlled for airdropped tokens
+ * (e.g. scam-URL token names). Publishing it as metadata poisoned
+ * `state.assetsInfo`, marking spam assets as "known" and exempting them from
+ * TokenDataSource's spam filtering on all subsequent updates. Metadata is
+ * intentionally left for TokenDataSource to resolve from the Token API — the
+ * metadata source of truth — during the same pipeline pass. The payload's
+ * `decimals` is still used locally to convert raw amounts to human-readable
+ * balances.
+ *
  * @param updates - Balance updates from account-activity websocket payload.
  * @param accountId - Internal account UUID.
- * @param getAssetType - Resolver for asset metadata type.
  * @returns DataResponse with merge mode when balances are present.
  */
 function processAccountActivityBalanceUpdates(
   updates: BalanceUpdate[],
   accountId: string,
-  getAssetType: (assetId: Caip19AssetId) => 'native' | 'erc20' | 'spl',
 ): DataResponse {
   const assetsBalance = Object.create(null) as Record<
     string,
@@ -54,10 +61,6 @@ function processAccountActivityBalanceUpdates(
     Caip19AssetId,
     AssetBalance
   >;
-  const assetsMetadata = Object.create(null) as Record<
-    Caip19AssetId,
-    AssetMetadata
-  >;
 
   for (const update of updates) {
     const { asset, postBalance } = update;
@@ -83,19 +86,11 @@ function processAccountActivityBalanceUpdates(
     assetsBalance[accountId][assetId] = {
       amount: humanReadableAmount,
     };
-
-    assetsMetadata[assetId] = {
-      type: getAssetType(assetId),
-      symbol: asset.unit,
-      name: asset.unit,
-      decimals: asset.decimals,
-    };
   }
 
   const response: DataResponse = { updateMode: 'merge' };
   if (Object.keys(assetsBalance[accountId]).length > 0) {
     response.assetsBalance = assetsBalance;
-    response.assetsInfo = assetsMetadata;
   }
 
   return response;
@@ -133,8 +128,6 @@ export type AccountActivityDataSourceOptions = {
     chains: ChainId[],
     previousChains: ChainId[],
   ) => void;
-  /** Returns the asset type ('native' | 'erc20' | 'spl') for a given CAIP-19 asset ID. */
-  getAssetType: (assetId: Caip19AssetId) => 'native' | 'erc20' | 'spl';
   /**
    * Pushes decoded balance updates to the controller (bound to
    * `AssetsController.handleAssetsUpdate`). AADS is event-driven and never
@@ -196,10 +189,6 @@ export class AccountActivityDataSource extends AbstractDataSource<
     previousChains: ChainId[],
   ) => void;
 
-  readonly #getAssetType: (
-    assetId: Caip19AssetId,
-  ) => 'native' | 'erc20' | 'spl';
-
   readonly #onAssetsUpdate: (
     response: DataResponse,
     request?: DataRequest,
@@ -219,7 +208,6 @@ export class AccountActivityDataSource extends AbstractDataSource<
 
     this.#messenger = options.messenger;
     this.#onActiveChainsUpdated = options.onActiveChainsUpdated;
-    this.#getAssetType = options.getAssetType;
     this.#onAssetsUpdate = options.onAssetsUpdate;
     this.#onBalanceUpdatedBound = this.#onBalanceUpdated.bind(this);
 
@@ -285,7 +273,6 @@ export class AccountActivityDataSource extends AbstractDataSource<
       const response = processAccountActivityBalanceUpdates(
         updates,
         account.id,
-        (assetId) => this.#getAssetType(assetId),
       );
 
       if (!response.assetsBalance) {