Don't allow overwriting non-objects

MetaMask · Dec 18, 2024 · 0c8be2d · 0c8be2d
1 parent d9c7cd8
commit 0c8be2d
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 6 deletions.
diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js
@@ -10,10 +10,10 @@ module.exports = deepmerge(baseConfig, {
   ],
   coverageThreshold: {
     global: {
-      branches: 93.28,
+      branches: 93.33,
       functions: 97.46,
       lines: 98.03,
-      statements: 97.61,
+      statements: 97.62,
     },
   },
 });
diff --git a/packages/snaps-rpc-methods/src/permitted/setState.test.ts b/packages/snaps-rpc-methods/src/permitted/setState.test.ts
@@ -487,6 +487,28 @@ describe('set', () => {
     });
   });
 
+  it('allows overwriting if a parent key is `null`', () => {
+    const object = {
+      nested: null,
+    };
+
+    expect(set(object, 'nested.key', 'newValue')).toStrictEqual({
+      nested: {
+        key: 'newValue',
+      },
+    });
+  });
+
+  it('throws if a parent key is not an object', () => {
+    const object = {
+      nested: 'value',
+    };
+
+    expect(() => set(object, 'nested.key', 'newValue')).toThrow(
+      'Invalid params: Cannot overwrite non-object value.',
+    );
+  });
+
   it('throws an error if the key is a prototype pollution attempt', () => {
     expect(() => set({}, '__proto__.polluted', 'value')).toThrow(
       'Invalid params: Key contains forbidden characters.',

diff --git a/packages/snaps-rpc-methods/src/permitted/setState.ts b/packages/snaps-rpc-methods/src/permitted/setState.ts
@@ -12,7 +12,13 @@ import {
   StructError,
 } from '@metamask/superstruct';
 import type { PendingJsonRpcResponse, JsonRpcRequest } from '@metamask/utils';
-import { isObject, assert, JsonStruct, type Json } from '@metamask/utils';
+import {
+  hasProperty,
+  isObject,
+  assert,
+  JsonStruct,
+  type Json,
+} from '@metamask/utils';
 
 import { manageStateBuilder } from '../restricted/manageState';
 import type { MethodHooksObject } from '../utils';
@@ -239,9 +245,18 @@ export function set(
       return requiredObject;
     }
 
-    currentObject[currentKey] = isObject(currentObject[currentKey])
-      ? currentObject[currentKey]
-      : {};
+    if (
+      !hasProperty(currentObject, currentKey) ||
+      currentObject[currentKey] === null
+    ) {
+      currentObject[currentKey] = {};
+    }
+
+    if (!isObject(currentObject[currentKey])) {
+      throw rpcErrors.invalidParams(
+        'Invalid params: Cannot overwrite non-object value.',
+      );
+    }
 
     currentObject = currentObject[currentKey] as Record<string, Json>;
   }