FIX: Validate Snap arguments (#7)

* Validate snap parameters * Fix formatting * Fix comment * Fix lint * fix lint * Fix typescript version * Add validation * Fix CI * Sig validation * fix snap shasum --------- Co-authored-by: Victor Lopez <[email protected]>
Sovereign-Labs · Dec 7, 2023 · 369ec86 · 369ec86
1 parent ecc22a3
commit 369ec86
Show file tree

Hide file tree

Showing 8 changed files with 32,517 additions and 3,341 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -48,7 +48,7 @@
     "prettier": "^2.7.1",
     "prettier-plugin-packagejson": "^2.2.18",
     "sharp": "^0.30.7",
-    "typescript": "^4.7.4"
+    "typescript": "4.8.4"
   },
   "packageManager": "[email protected]",
   "engines": {

diff --git a/packages/snap/package.json b/packages/snap/package.json
@@ -31,9 +31,11 @@
     "@metamask/rpc-errors": "^6.1.0",
     "@metamask/snaps-types": "^3.0.1",
     "@metamask/snaps-ui": "^3.0.1",
+    "@metamask/snaps-utils": "^5.0.0",
     "@metamask/utils": "^8.2.1",
     "@noble/ed25519": "^1.6.0",
-    "buffer": "^6.0.3"
+    "buffer": "^6.0.3",
+    "superstruct": "^1.0.3"
   },
   "devDependencies": {
     "@jest/globals": "^29.5.0",
@@ -60,7 +62,7 @@
     "rimraf": "^3.0.2",
     "through2": "^4.0.2",
     "ts-jest": "^29.1.0",
-    "typescript": "^4.7.4"
+    "typescript": "4.8.4"
   },
   "packageManager": "[email protected]",
   "engines": {

diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/Sovereign-Labs/sov-snap.git"
   },
   "source": {
-    "shasum": "ZcjIMDNneWe6FWkEl1qAXeyFYYTAwtJ+GuCnp6JKoMk=",
+    "shasum": "4o0iXdvcsoPhEgfJ41LTGihawXvb1EHJ67P7GNnXgbk=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/snap/src/index.ts b/packages/snap/src/index.ts
@@ -1,12 +1,13 @@
 import { SLIP10Node } from '@metamask/key-tree';
-import { providerErrors } from '@metamask/rpc-errors';
+import { providerErrors, rpcErrors } from '@metamask/rpc-errors';
 import type { OnRpcRequestHandler } from '@metamask/snaps-types';
 import { DialogType } from '@metamask/snaps-types';
 import { copyable, heading, panel, text } from '@metamask/snaps-ui';
 import { add0x, assert, bytesToHex, remove0x } from '@metamask/utils';
 import { sign } from '@noble/ed25519';
+import { validate as superstructValidate } from 'superstruct';
 
-import type { GetBip32PublicKeyParams, SignTransactionParams } from './types';
+import { GetBip32PublicKeyParamsStruct, SignTransactionStruct } from './types';
 import { SovWasm } from './wasm';
 
 const wasm = new SovWasm();
@@ -29,8 +30,16 @@ export const onRpcRequest: OnRpcRequestHandler = async ({
     // the return is a plain hex string
     // https://docs.metamask.io/snaps/reference/rpc-api/#returns-5
     case 'getPublicKey': {
-      const { path, compressed } = request.params as GetBip32PublicKeyParams;
+      const [validationErr, params] = superstructValidate(
+        request.params,
+        GetBip32PublicKeyParamsStruct,
+      );
 
+      if (validationErr !== undefined) {
+        throw rpcErrors.invalidParams(validationErr.toString());
+      }
+
+      const { path, compressed } = params;
       // eslint-disable-next-line @typescript-eslint/await-thenable
       const approved = await snap.request({
         method: 'snap_dialog',
@@ -44,7 +53,6 @@ export const onRpcRequest: OnRpcRequestHandler = async ({
           ]),
         },
       });
-
       if (!approved) {
         throw providerErrors.userRejectedRequest();
       }
@@ -61,7 +69,16 @@ export const onRpcRequest: OnRpcRequestHandler = async ({
     }
 
     case 'signTransaction': {
-      const { transaction, path } = request.params as SignTransactionParams;
+      const [validationErr, params] = superstructValidate(
+        request.params,
+        SignTransactionStruct,
+      );
+
+      if (validationErr !== undefined) {
+        throw rpcErrors.invalidParams(validationErr.toString());
+      }
+
+      const { transaction, path } = params;
 
       try {
         const call = wasm.serializeCall(transaction.message, transaction.nonce);
@@ -113,7 +130,6 @@ export const onRpcRequest: OnRpcRequestHandler = async ({
         const txHex = bytesToHex(tx);
 
         wasm.dealloc();
-
         return txHex;
       } catch (er) {
         wasm.dealloc();

diff --git a/packages/snap/src/types.ts b/packages/snap/src/types.ts
@@ -1,61 +1,56 @@
+import { Bip32PathStruct } from '@metamask/snaps-utils';
+import {
+  boolean,
+  object,
+  optional,
+  type,
+  string,
+  number,
+  array,
+} from 'superstruct';
+
 /**
- * The parameters for calling the `getPublicKey` JSON-RPC method.
- *
- * Note: For simplicity, these are not validated by the snap. In production, you
- * should validate that the request object matches this type before using it.
+ * `type` is used instead of `object` to allow unknown properties.
  */
-export type GetBip32PublicKeyParams = {
+export const GetBip32PublicKeyParamsStruct = type({
   /**
    * The BIP-32 path to the account.
    */
-  path: ['m', ...(`${number}` | `${number}'`)[]];
-
+  path: Bip32PathStruct,
   /**
    * Whether to return the public key in compressed form.
    */
-  compressed?: boolean | undefined;
-
-  /**
-   * Miscellaneous parameters, which are passed to `snap_getBip32PublicKey`.
-   */
-  [key: string]: unknown;
-};
+  compressed: optional(boolean()),
+});
 
 /**
  * The transaction object to be submitted by the UI so the signature can be generated.
- *
- * Note: For simplicity, these are not validated by the snap. In production, you
- * should validate that the request object matches this type before using it.
  */
-export type Transaction = {
+export const TransactionStruct = object({
   /**
    * The JSON transaction to sign.
    */
-  message: string;
-
+  message: string(),
   /**
    * The nonce for the transaction signature.
    */
-  nonce: number;
-};
+  nonce: number(),
+});
 
 /**
  * The parameters for calling the `signTransaction` JSON-RPC method.
- *
- * Note: For simplicity, these are not validated by the snap. In production, you
- * should validate that the request object matches this type before using it.
  */
-export type SignTransactionParams = {
+export const SignTransactionStruct = object({
   /**
    * The JSON transaction to sign.
    */
-  transaction: Transaction;
+  transaction: TransactionStruct,
 
   /**
    * The BIP-32 path to the account.
    */
-  path: string[];
-};
+  path: array(string()),
+});
 
 /**
  * The expected WASM interface from the imported module.

diff --git a/packages/snap/tsconfig.json b/packages/snap/tsconfig.json
@@ -1,6 +1,7 @@
 {
   "extends": "../../tsconfig.json",
   "compilerOptions": {
+    "strictNullChecks": true,
     "baseUrl": "./"
   },
   "include": ["**/*.ts"]