diff --git a/packages/manager/.changeset/pr-10814-tech-stories-1724434847753.md b/packages/manager/.changeset/pr-10814-tech-stories-1724434847753.md new file mode 100644 index 00000000000..cbe20d384c3 --- /dev/null +++ b/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)) diff --git a/packages/manager/package.json b/packages/manager/package.json index 0610dbe4518..22afe983256 100644 --- a/packages/manager/package.json +++ b/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", diff --git a/packages/manager/src/features/Linodes/LinodeCreatev2/utilities.ts b/packages/manager/src/features/Linodes/LinodeCreatev2/utilities.ts index 631d6f417cb..5679f43bc84 100644 --- a/packages/manager/src/features/Linodes/LinodeCreatev2/utilities.ts +++ b/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', diff --git a/packages/manager/src/utilities/formikErrorUtils.test.ts b/packages/manager/src/utilities/formikErrorUtils.test.ts index 3b7df4ce74b..48adf78a959 100644 --- a/packages/manager/src/utilities/formikErrorUtils.test.ts +++ b/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', + }); + }); + }); +}); diff --git a/packages/manager/src/utilities/formikErrorUtils.ts b/packages/manager/src/utilities/formikErrorUtils.ts index 3e4e14deea4..1f46a13d6ea 100644 --- a/packages/manager/src/utilities/formikErrorUtils.ts +++ b/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 = ( }, {}); }; +// 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 = ( + obj: FormikErrors, + path: string, + value: string +): FormikErrors => { + 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, 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[] = [] diff --git a/yarn.lock b/yarn.lock index 9e5d35920ae..18ac5e1751a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3912,13 +3912,6 @@ dependencies: "@types/lodash" "*" -"@types/lodash.set@^4.3.9": - version "4.3.9" - resolved "https://registry.yarnpkg.com/@types/lodash.set/-/lodash.set-4.3.9.tgz#55d95bce407b42c6655f29b2d0811fd428e698f0" - integrity sha512-KOxyNkZpbaggVmqbpr82N2tDVTx05/3/j0f50Es1prxrWB0XYf9p3QNxqcbWb7P1Q9wlvsUSlCFnwlPCIJ46PQ== - dependencies: - "@types/lodash" "*" - "@types/lodash@*": version "4.17.0" resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.17.0.tgz#d774355e41f372d5350a4d0714abb48194a489c3" @@ -9179,11 +9172,6 @@ lodash.once@^4.1.1: resolved "https://registry.yarnpkg.com/lodash.once/-/lodash.once-4.1.1.tgz#0dd3971213c7c56df880977d504c88fb471a97ac" integrity sha512-Sb487aTOCr9drQVL8pIxOzVhafOjZN9UU54hiN8PU3uAiSV7lx1yYNpbNmex2PK6dSJoNTSJUUswT651yww3Mg== -lodash.set@^4.3.2: - version "4.3.2" - resolved "https://registry.yarnpkg.com/lodash.set/-/lodash.set-4.3.2.tgz#d8757b1da807dde24816b0d6a84bea1a76230b23" - integrity sha512-4hNPN5jlm/N/HLMCO43v8BXKq9Z7QdAGc/VGrRD61w8gN9g/6jF9A4L1pbUgBLCffi0w9VsXfTOij5x8iTyFvg== - lodash.sortby@^4.7.0: version "4.7.0" resolved "https://registry.yarnpkg.com/lodash.sortby/-/lodash.sortby-4.7.0.tgz#edd14c824e2cc9c1e0b0a1b42bb5210516a42438"