From 3960fdfcf91b4e219b4b9b0ea3438a055f6769f3 Mon Sep 17 00:00:00 2001
From: Alex Donesky <adonesky@gmail.com>
Date: Mon, 26 Jun 2023 09:12:13 -0500
Subject: [PATCH] Add `tokenId` type validation in `wallet_watchAsset`
 middleware (#19738)

---
 .../handlers/watch-asset.js                   | 17 ++++
 .../handlers/watch-asset.test.js              | 99 +++++++++++++++++++
 package.json                                  |  2 +-
 yarn.lock                                     | 10 +-
 4 files changed, 122 insertions(+), 6 deletions(-)
 create mode 100644 app/scripts/lib/rpc-method-middleware/handlers/watch-asset.test.js

diff --git a/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js b/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js
index 8954a15f5..2f7fb5d7a 100644
--- a/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js
+++ b/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js
@@ -1,3 +1,5 @@
+import { ERC1155, ERC721 } from '@metamask/controller-utils';
+import { ethErrors } from 'eth-rpc-errors';
 import { MESSAGE_TYPE } from '../../../../../shared/constants/app';
 
 const watchAsset = {
@@ -39,6 +41,21 @@ async function watchAssetHandler(
       params: { options: asset, type },
       origin,
     } = req;
+
+    const { tokenId } = asset;
+
+    if (
+      [ERC721, ERC1155].includes(type) &&
+      tokenId !== undefined &&
+      typeof tokenId !== 'string'
+    ) {
+      return end(
+        ethErrors.rpc.invalidParams({
+          message: `Expected parameter 'tokenId' to be type 'string'. Received type '${typeof tokenId}'`,
+        }),
+      );
+    }
+
     await handleWatchAssetRequest(asset, type, origin);
     res.result = true;
     return end();
diff --git a/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.test.js b/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.test.js
new file mode 100644
index 000000000..5af3ad1bd
--- /dev/null
+++ b/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.test.js
@@ -0,0 +1,99 @@
+import { ERC20, ERC721 } from '@metamask/controller-utils';
+import { ethErrors } from 'eth-rpc-errors';
+import watchAssetHandler from './watch-asset';
+
+describe('watchAssetHandler', () => {
+  let mockEnd;
+  let mockHandleWatchAssetRequest;
+
+  beforeEach(() => {
+    mockEnd = jest.fn();
+    mockHandleWatchAssetRequest = jest.fn();
+  });
+
+  it('should handle valid input for type ERC721 correctly', async () => {
+    const req = {
+      params: {
+        options: {
+          address: '0x1234',
+          tokenId: 'testTokenId',
+        },
+        type: ERC721,
+      },
+      origin: 'testOrigin',
+    };
+
+    const res = {
+      result: false,
+    };
+
+    await watchAssetHandler.implementation(req, res, null, mockEnd, {
+      handleWatchAssetRequest: mockHandleWatchAssetRequest,
+    });
+
+    expect(mockHandleWatchAssetRequest).toHaveBeenCalledWith(
+      req.params.options,
+      req.params.type,
+      req.origin,
+    );
+    expect(res.result).toStrictEqual(true);
+    expect(mockEnd).toHaveBeenCalledWith();
+  });
+
+  it('should handle valid input for type ERC20 correctly', async () => {
+    const req = {
+      params: {
+        options: {
+          address: '0x1234',
+          symbol: 'TEST',
+          decimals: 18,
+        },
+        type: ERC20,
+      },
+      origin: 'testOrigin',
+    };
+
+    const res = {
+      result: false,
+    };
+
+    await watchAssetHandler.implementation(req, res, null, mockEnd, {
+      handleWatchAssetRequest: mockHandleWatchAssetRequest,
+    });
+
+    expect(mockHandleWatchAssetRequest).toHaveBeenCalledWith(
+      req.params.options,
+      req.params.type,
+      req.origin,
+    );
+    expect(res.result).toStrictEqual(true);
+    expect(mockEnd).toHaveBeenCalledWith();
+  });
+
+  it('should throw when type is ERC721 and tokenId type is invalid', async () => {
+    const req = {
+      params: {
+        options: {
+          address: '0x1234',
+          tokenId: 222,
+        },
+        type: ERC721,
+      },
+      origin: 'testOrigin',
+    };
+
+    const res = {
+      result: false,
+    };
+
+    await watchAssetHandler.implementation(req, res, null, mockEnd, {
+      handleWatchAssetRequest: mockHandleWatchAssetRequest,
+    });
+
+    expect(mockEnd).toHaveBeenCalledWith(
+      ethErrors.rpc.invalidParams({
+        message: `Expected parameter 'tokenId' to be type 'string'. Received type 'number'`,
+      }),
+    );
+  });
+});
diff --git a/package.json b/package.json
index adf1a8995..fff0ea2f6 100644
--- a/package.json
+++ b/package.json
@@ -386,7 +386,7 @@
     "@metamask/eslint-config-typescript": "^9.0.1",
     "@metamask/forwarder": "^1.1.0",
     "@metamask/phishing-warning": "^2.1.0",
-    "@metamask/test-dapp": "^7.0.0",
+    "@metamask/test-dapp": "^7.0.1",
     "@sentry/cli": "^1.58.0",
     "@storybook/addon-a11y": "^7.0.11",
     "@storybook/addon-actions": "^7.0.11",
diff --git a/yarn.lock b/yarn.lock
index 56345e62f..8f80b5875 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -4931,10 +4931,10 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@metamask/test-dapp@npm:^7.0.0":
-  version: 7.0.0
-  resolution: "@metamask/test-dapp@npm:7.0.0"
-  checksum: 4a03ed86a97c94eef8131c0951fb7bf7eb0c988c407ae8e5ed6a897da30d1c48adf3e1e51e45220df6e5d51c22736a06c732ac95ba5741223573cd660706876b
+"@metamask/test-dapp@npm:^7.0.1":
+  version: 7.0.1
+  resolution: "@metamask/test-dapp@npm:7.0.1"
+  checksum: cbd9f7d66c32ecdccc2039b9d60ef7c0a118ce22392f2eec2ffc0911d3314f75eb81fc9bc6cdf14efd34de5fffb013693344ca2a2e46aa0a8cc33a3a8d8b0fea
   languageName: node
   linkType: hard
 
@@ -24367,7 +24367,7 @@ __metadata:
     "@metamask/snaps-utils-flask": "npm:@metamask/snaps-utils@0.34.0-flask.1"
     "@metamask/subject-metadata-controller": ^2.0.0
     "@metamask/swappable-obj-proxy": ^2.1.0
-    "@metamask/test-dapp": ^7.0.0
+    "@metamask/test-dapp": ^7.0.1
     "@metamask/utils": ^5.0.0
     "@ngraveio/bc-ur": ^1.1.6
     "@popperjs/core": ^2.4.0