tech story: [M3-8385] - Replace lodash set utility function to handle…

… prototype pollution security threat (linode#10814)

* remove usage of omit that depends on lodash

* it is not going

* need to figure out types

* hopefully this is something

* todo: fix all bugs and write all the tests

* write tests - need to debug

* determine if supposed index is actually a valid index

* some path indexing that seems difficult to pursue

* update naming, tests

* remove set from package.json

* add back in some commented out tests

* some cleanup, need to look over everything

* update test cases and comments

* changeset + update tests

* simplify set, remove separate set utility files

* Update packages/manager/src/utilities/formikErrorUtils.ts

Co-authored-by: Alban Bailly <130582365+abailly-akamai@users.noreply.github.com>

* update types + fix tests  @abailly-akamai

---------

Co-authored-by: Alban Bailly <130582365+abailly-akamai@users.noreply.github.com>

packages/manager/.changeset/pr-10814-tech-stories-1724434847753.md

@@ -0,0 +1,5 @@
+---
+"@linode/manager": Tech Stories
+---
+
+Replace lodash set utility function to handle security threat raised by Dependabot ([#10814](https://github.com/linode/manager/pull/10814))

packages/manager/package.json

@@ -49,7 +49,6 @@
     "libphonenumber-js": "^1.10.6",
     "lodash.clonedeep": "^4.5.0",
     "lodash.curry": "^4.1.1",
-    "lodash.set": "^4.3.2",
     "logic-query-parser": "^0.0.5",
     "luxon": "3.4.4",
     "markdown-it": "^12.3.2",
@@ -143,7 +142,6 @@
     "@types/jspdf": "^1.3.3",
     "@types/lodash.clonedeep": "^4.5.9",
     "@types/lodash.curry": "^4.1.9",
-    "@types/lodash.set": "^4.3.9",
     "@types/luxon": "3.4.2",
     "@types/markdown-it": "^10.0.2",
     "@types/md5": "^2.1.32",

packages/manager/src/features/Linodes/LinodeCreatev2/utilities.ts

@@ -1,4 +1,3 @@
-import { omit } from 'lodash';
 import { useCallback } from 'react';
 import { useHistory } from 'react-router-dom';
 
@@ -9,6 +8,7 @@ import { sendCreateLinodeEvent } from 'src/utilities/analytics/customEventAnalyt
 import { sendLinodeCreateFormErrorEvent } from 'src/utilities/analytics/formEventAnalytics';
 import { privateIPRegex } from 'src/utilities/ipUtils';
 import { isNotNullOrUndefined } from 'src/utilities/nullOrUndefined';
+import { omitProps } from 'src/utilities/omittedProps';
 import { getQueryParamsFromQueryString } from 'src/utilities/queryParams';
 
 import { utoa } from '../LinodesCreate/utilities';
@@ -148,7 +148,7 @@ export const tabs: LinodeCreateType[] = [
 export const getLinodeCreatePayload = (
   formValues: LinodeCreateFormValues
 ): CreateLinodeRequest => {
-  const values = omit(formValues, [
+  const values = omitProps(formValues, [
     'linode',
     'hasSignedEUAgreement',
     'firewallOverride',

packages/manager/src/utilities/formikErrorUtils.test.ts

@@ -2,6 +2,7 @@ import {
   getFormikErrorsFromAPIErrors,
   handleAPIErrors,
   handleVPCAndSubnetErrors,
+  set,
 } from './formikErrorUtils';
 
 const errorWithoutField = [{ reason: 'Internal server error' }];
@@ -35,51 +36,51 @@ describe('handleAPIErrors', () => {
 
 const subnetMultipleErrorsPerField = [
   {
-    reason: 'not expected error for label',
     field: 'subnets[0].label',
+    reason: 'not expected error for label',
   },
   {
-    reason: 'expected error for label',
     field: 'subnets[0].label',
+    reason: 'expected error for label',
   },
   {
-    reason: 'not expected error for label',
     field: 'subnets[3].label',
+    reason: 'not expected error for label',
   },
   {
-    reason: 'expected error for label',
     field: 'subnets[3].label',
+    reason: 'expected error for label',
   },
   {
-    reason: 'not expected error for ipv4',
     field: 'subnets[3].ipv4',
+    reason: 'not expected error for ipv4',
   },
   {
-    reason: 'expected error for ipv4',
     field: 'subnets[3].ipv4',
+    reason: 'expected error for ipv4',
   },
 ];
 
 const subnetErrors = [
   {
-    reason: 'Label required',
     field: 'subnets[1].label',
+    reason: 'Label required',
   },
   {
-    reason: 'bad label',
     field: 'subnets[2].label',
+    reason: 'bad label',
   },
   {
-    reason: 'cidr ipv4',
     field: 'subnets[2].ipv4',
+    reason: 'cidr ipv4',
   },
   {
-    reason: 'needs an ip',
     field: 'subnets[4].ipv4',
+    reason: 'needs an ip',
   },
   {
-    reason: 'needs an ipv6',
     field: 'subnets[4].ipv6',
+    reason: 'needs an ipv6',
   },
 ];
 
@@ -93,7 +94,7 @@ describe('handleVpcAndConvertSubnetErrors', () => {
     expect(Object.keys(errors)).toHaveLength(3);
     expect(Object.keys(errors)).toEqual(['1', '2', '4']);
     expect(errors[1]).toEqual({ label: 'Label required' });
-    expect(errors[2]).toEqual({ label: 'bad label', ipv4: 'cidr ipv4' });
+    expect(errors[2]).toEqual({ ipv4: 'cidr ipv4', label: 'bad label' });
     expect(errors[4]).toEqual({ ipv4: 'needs an ip', ipv6: 'needs an ipv6' });
   });
 
@@ -106,8 +107,8 @@ describe('handleVpcAndConvertSubnetErrors', () => {
     expect(Object.keys(errors)).toHaveLength(2);
     expect(errors[0]).toEqual({ label: 'expected error for label' });
     expect(errors[3]).toEqual({
-      label: 'expected error for label',
       ipv4: 'expected error for ipv4',
+      label: 'expected error for label',
     });
   });
 
@@ -201,3 +202,191 @@ describe('getFormikErrorsFromAPIErrors', () => {
     }
   });
 });
+
+describe('Tests for set', () => {
+  it("returns the passed in 'object' as is if it's not actually a (non array) object", () => {
+    expect(set([], 'path not needed', '1')).toEqual([]);
+  });
+
+  describe('Correctly setting the value at the given path', () => {
+    it('sets the value for a simple path', () => {
+      const object = {};
+      let settedObject = set(object, 'test', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({ test: '1' });
+
+      settedObject = set(object, 'test2', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({ test: '1', test2: '1' });
+    });
+
+    it('sets the value for complex string paths (without indexes)', () => {
+      const object = {};
+
+      set(object, 'a.b.c', 'c');
+      expect(object).toEqual({ a: { b: { c: 'c' } } });
+
+      set(object, 'a.b.d', 'd');
+      expect(object).toEqual({
+        a: { b: { c: 'c', d: 'd' } },
+      });
+
+      set(object, 'e[f][g]', 'g');
+      expect(object).toEqual({
+        a: { b: { c: 'c', d: 'd' } },
+        e: { f: { g: 'g' } },
+      });
+    });
+
+    it('sets the value for complex string paths (with indexes)', () => {
+      const object = {};
+
+      set(object, 'a.b.1', 'b1');
+      expect(object).toEqual({ a: { b: [undefined, 'b1'] } });
+      set(object, 'a.b[0]', '5');
+      expect(object).toEqual({ a: { b: ['5', 'b1'] } });
+
+      set(object, 'a.b.2', 'b2');
+      expect(object).toEqual({
+        a: { b: ['5', 'b1', 'b2'] },
+      });
+
+      set(object, 'a.b[3].c', 'c');
+      expect(object).toEqual({
+        a: { b: ['5', 'b1', 'b2', { c: 'c' }] },
+      });
+    });
+
+    it('only considers 0 or positive integers for setting array values', () => {
+      const object = {};
+
+      expect(set(object, 'test[-01].test1', 'test')).toEqual({
+        test: { '-01': { test1: 'test' } },
+      });
+      expect(set(object, 'test[-01][-02]', 'test2')).toEqual({
+        test: { '-01': { '-02': 'test2', test1: 'test' } },
+      });
+      expect(set(object, 'test[   02]', 'test3')).toEqual({
+        test: {
+          '   02': 'test3',
+          '-01': { '-02': 'test2', test1: 'test' },
+        },
+      });
+      expect(set(object, 'test[0 0]', 'test4')).toEqual({
+        test: {
+          '   02': 'test3',
+          '-01': { '-02': 'test2', test1: 'test' },
+          '0 0': 'test4',
+        },
+      });
+    });
+
+    it('considers numbers as keys if they are not followed by another number', () => {
+      const object = {};
+      set(object, '1', 'test');
+      expect(object).toEqual({ 1: 'test' });
+
+      set(object, '2', '2');
+      expect(object).toEqual({ 1: 'test', 2: '2' });
+    });
+
+    it('treats numbers as array indexes if they precede some previous key (if they are valid integers >= 0)', () => {
+      const obj1 = set({}, '1[1]', 'test');
+      expect(obj1).toEqual({ 1: [undefined, 'test'] });
+
+      const obj2 = set({}, '1.2', 'test');
+      expect(obj2).toEqual({ 1: [undefined, undefined, 'test'] });
+    });
+
+    it('can replace the value at an already existing key', () => {
+      const alreadyExisting = { test: 'test' };
+      expect(set(alreadyExisting, 'test', 'changed')).toEqual({
+        test: 'changed',
+      });
+      expect(set(alreadyExisting, 'test[test2][test3]', 'changed x4')).toEqual({
+        test: { test2: { test3: 'changed x4' } },
+      });
+    });
+
+    it('sets the value for nonstandard paths', () => {
+      expect(set({}, 'test.[.test]', 'testing 2')).toEqual({
+        test: { test: 'testing 2' },
+      });
+      expect(set({}, 'test.[te[st]', 'testing 3')).toEqual({
+        test: { te: { st: 'testing 3' } },
+      });
+      expect(set({}, 'test.]test', 'testing 4')).toEqual({
+        test: { test: 'testing 4' },
+      });
+    });
+  });
+
+  describe('Ensuring safety against prototype pollution and that the passed in and returned object are the same', () => {
+    it('protects against the given string path matching a prototype pollution key', () => {
+      const object = {};
+      // __proto__
+      let settedObject = set(object, '__proto__', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({});
+
+      // constructor
+      settedObject = set(object, 'constructor', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({});
+
+      // prototype
+      settedObject = set(object, 'prototype', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({});
+    });
+
+    it('protects against the given string path containing prototype pollution keys that are separated by path delimiters', () => {
+      const object = {};
+      // prototype pollution key separated by .
+      let settedObject = set(object, 'test.__proto__.test', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({});
+
+      settedObject = set(object, 'test.constructor.test', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({});
+
+      settedObject = set(object, 'test.prototype.test', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({});
+
+      // prototype pollution key separated by []
+      settedObject = set(object, 'test.test[__proto__]', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({});
+
+      settedObject = set(object, 'test.test[constructor]', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({});
+
+      settedObject = set(object, 'test.test[prototype]', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({});
+    });
+
+    it('is not considered prototype pollution if the string paths have a key not separated by delimiters', () => {
+      const object = {};
+      // prototype pollution key separated by .
+      let settedObject = set(object, 'test__proto__test', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({ test__proto__test: '1' });
+
+      settedObject = set(object, 'constructortest', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({ constructortest: '1', test__proto__test: '1' });
+
+      settedObject = set(object, 'testprototype', '1');
+      expect(object).toBe(settedObject);
+      expect(object).toEqual({
+        constructortest: '1',
+        test__proto__test: '1',
+        testprototype: '1',
+      });
+    });
+  });
+});

packages/manager/src/utilities/formikErrorUtils.ts

@@ -1,4 +1,3 @@
-import set from 'lodash.set';
 import { reverse } from 'ramda';
 
 import { getAPIErrorOrDefault } from './errorUtils';
@@ -23,6 +22,67 @@ export const getFormikErrorsFromAPIErrors = <T>(
   }, {});
 };
 
+// regex used in the below set function
+const onlyDigitsRegex = /^\d+$/;
+
+/**
+ * Helper for getFormikErrorsFromAPIErrors, sets the given value at a specified path of the given object.
+ * Note that while we are using this function in place of lodash's set, it is not an exact replacement.
+ * This method both mutates the passed in object and returns it.
+ *
+ * @param object — The object to modify.
+ * @param path — The path of the property to set.
+ * @param value — The value to set.
+ * @return — Returns object.
+ */
+export const set = <T>(
+  obj: FormikErrors<T>,
+  path: string,
+  value: string
+): FormikErrors<T> => {
+  const parts = path.split(/\.|\[|\]/).filter(Boolean);
+
+  // ensure that obj is not an array and that the path is prototype pollution safe
+  if (Array.isArray(obj) || !isPrototypePollutionSafe(parts)) {
+    return obj;
+  }
+
+  parts.reduce((acc: Record<string, unknown>, part: string, index: number) => {
+    if (index === parts.length - 1) {
+      // Last part, set the value
+      acc[part] = value;
+    } else if (part.match(onlyDigitsRegex)) {
+      // Handle array indices
+      const arrayIndex = parseInt(part, 10);
+      acc[arrayIndex] =
+        acc[arrayIndex] ?? (parts[index + 1].match(onlyDigitsRegex) ? [] : {});
+    } else {
+      // Handle nested objects
+      const potentialNextVal = parts[index + 1].match(onlyDigitsRegex)
+        ? []
+        : {};
+      acc[part] = typeof acc[part] === 'object' ? acc[part] : potentialNextVal;
+    }
+    return acc[part];
+  }, obj);
+
+  return obj;
+};
+
+/**
+ * Ensures a path cannot lead to a prototype pollution issue.
+ *
+ * @param path - The path to check
+ * @return - boolean depending on whether the path is safe or not
+ */
+const isPrototypePollutionSafe = (path: string[]): boolean => {
+  return path.reduce((safeSoFar, val) => {
+    const isCurKeySafe =
+      val !== '__proto__' && val !== 'prototype' && val !== 'constructor';
+    return safeSoFar && isCurKeySafe;
+  }, true);
+};
+
 export const handleFieldErrors = (
   callback: (error: unknown) => void,
   fieldErrors: APIError[] = []