Merge pull request #12 from clickbar/fix/prototype-pollution

Add guards against prototype pollution

CHANGELOG.md

@@ -7,6 +7,7 @@ All notable changes to **dot-diver** will be documented here. Inspired by [keep
 - Updated dependencies
 - Formatted code with new lint rules
 - Fixed testcase for new TypeScript behavior
+- Added guards against prototype pollution, thanks to @d3ng03 (<https://github.com/clickbar/dot-diver/security/advisories/GHSA-9w5f-mw3p-pj47>)
 
 ## [1.0.1](https://github.com/clickbar/dot-diver/tree/1.0.1) (2023-03-26)
 

src/index.ts

@@ -218,8 +218,12 @@ type PathValueEntry<T, P extends PathEntry<T, Depth>, Depth extends number = 10>
 
 type SearchableObject = Record<never, never> | unknown[]
 
+// eslint-disable-next-line @typescript-eslint/unbound-method
+const hasOwnProperty = Object.prototype.hasOwnProperty
+
 /**
- * Retrives a value from an object by dot notation
+ * Retrieves a value from an object by dot notation. The value is received by optional chaining,
+ * therefore this function returns undefined if an intermediate property is undefined.
  *
  * @param object - object to get value from
  * @param path - path to value
@@ -236,16 +240,27 @@ function getByPath<T extends SearchableObject, P extends PathEntry<T> & string>(
 ): PathValueEntry<T, P> {
   const pathArray = (path as string).split('.')
 
-  // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-return
-  return pathArray.reduce((accumulator: any, current) => accumulator?.[current], object)
+  // eslint-disable-next-line @typescript-eslint/no-unsafe-return
+  return pathArray.reduce((current: any, pathPart) => {
+    if (typeof current !== 'object' || !hasOwnProperty.call(current, pathPart)) {
+      return undefined
+    }
+
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-return
+    return current?.[pathPart]
+  }, object)
 }
 
 /**
- * Sets a value in an object by dot notation
+ * Sets a value in an object by dot notation. If an intermediate property is undefined,
+ * this function will throw an error.
+ *
  * @param object - object to set value in
  * @param path - path to value
  * @param value - value to set
  *
+ * @throws {Error} - if an intermediate property is undefined
+ *
  * @privateRemarks
  * The intersection between PathEntry<T, 10>  and string is necessary for TypeScript to successfully narrow down the type of P based on the user-provided path input.
  * Without the intersection, the path would just be of type PathEntry<T, 10> and PathValueEntry would be a union of all possible return types.
@@ -264,18 +279,24 @@ function setByPath<
   }
 
   // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
-  const objectToSet = pathArray.reduce(
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-return
-    (accumulator: any, current) => accumulator?.[current],
-    object,
-  )
+  const parentObject = pathArray.reduce((current: any, pathPart) => {
+    if (typeof current !== 'object' || !hasOwnProperty.call(current, pathPart)) {
+      throw new Error(`Property ${pathPart} is undefined`)
+    }
 
-  if (objectToSet === undefined) {
-    throw new Error('Path is invalid')
-  }
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
+    const next = current?.[pathPart]
+
+    if (next === undefined || next === null) {
+      throw new Error(`Property ${pathPart} is undefined`)
+    }
+
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-return
+    return next
+  }, object)
 
   // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-  objectToSet[lastKey] = value
+  parentObject[lastKey] = value
 }
 
 export type { PathEntry as Path, PathValueEntry as PathValue, SearchableObject }

test/index.test.ts

@@ -160,3 +160,58 @@ it('Test readme usage example: ⚙️ Customizing the Depth Limit', () => {
   setByPathDepth5(object, 'f.1.g', 'new array-item-2')
   expect(object.f[1].g).toBe('new array-item-2')
 })
+
+it('Test for prototype pollution', () => {
+  const object = {}
+
+  expect(() => {
+    // @ts-expect-error - this is not a valid path for the object
+    setByPath(object, '__proto__.polluted', true)
+  }).toThrowError('__proto__')
+
+  // @ts-expect-error - this is not a valid path for the object
+  expect(getByPath(object, '__proto__')).toBe(undefined)
+
+  expect(() => {
+    // @ts-expect-error - this is not a valid path for the object
+    setByPath(object, 'constructor.polluted', true)
+  }).toThrowError('constructor')
+
+  // @ts-expect-error - this is not a valid path for the object
+  expect(getByPath(object, 'constructor')).toBe(undefined)
+
+  // @ts-expect-error - this is should not be defined on the object
+  expect(object.polluted).toBe(undefined)
+
+  const object2 = { constructor: { prototype: { polluted: true } } }
+
+  expect(getByPath(object2, 'constructor.prototype.polluted')).toBe(true)
+
+  setByPath(object2, 'constructor.prototype.polluted', false)
+
+  expect(object2.constructor.prototype.polluted).toBe(false)
+
+  // eslint-disable-next-line @typescript-eslint/no-extraneous-class
+  const testClass = class TestClass {
+    // eslint-disable-next-line @typescript-eslint/no-useless-constructor, @typescript-eslint/no-empty-function
+    constructor() {}
+  }
+
+  const object3 = new testClass()
+
+  // @ts-expect-error - this is not a valid path for the object
+  expect(getByPath(object3, 'constructor.prototype')).toBe(undefined)
+
+  // @ts-expect-error - this is not a valid path for the object
+  expect(getByPath(object3, 'constructor')).toBe(undefined)
+
+  expect(() => {
+    // @ts-expect-error - this is not a valid path for the object
+    setByPath(object3, 'constructor.polluted', true)
+  }).toThrowError('constructor')
+
+  expect(() => {
+    // @ts-expect-error - this is not a valid path for the object
+    setByPath(object3, '__proto__.polluted', true)
+  }).toThrowError('__proto__')
+})