feat: add more control through configuration @ ParseJSONResultsPlugin. (#1453)

Author: igalklebanov

package.json

@@ -114,6 +114,7 @@
     "@types/better-sqlite3": "^7.6.13",
     "@types/chai": "^5.2.3",
     "@types/chai-as-promised": "^8.0.2",
+    "@types/prototype-pollution-vulnerable-lodash.merge-dont-upgrade": "npm:@types/lodash.merge@4.6.1",
     "@types/mocha": "^10.0.10",
     "@types/node": "^24.10.1",
     "@types/pg": "^8.15.6",
@@ -129,6 +130,7 @@
     "mocha": "^11.7.5",
     "mysql2": "^3.15.3",
     "pathe": "^2.0.3",
+    "prototype-pollution-vulnerable-lodash.merge-dont-upgrade": "npm:lodash.merge@4.6.1",
     "pg": "^8.15.6",
     "pg-cursor": "^2.14.6",
     "pkg-types": "^2.3.0",

pnpm-lock.yaml

@@ -1,6 +1,6 @@
 import type { QueryResult } from '../../driver/database-connection.js'
 import type { RootOperationNode } from '../../query-compiler/query-compiler.js'
-import { isPlainObject, isString } from '../../util/object-utils.js'
+import { freeze, isPlainObject, isString } from '../../util/object-utils.js'
 import type { UnknownRow } from '../../util/type-utils.js'
 import type {
   KyselyPlugin,
@@ -9,6 +9,17 @@ import type {
 } from '../kysely-plugin.js'
 
 export interface ParseJSONResultsPluginOptions {
+  /**
+   * A function that returns `true` if the given string is a JSON string that should be parsed. If a detected JSON string fails to parse, an error is thrown.
+   *
+   * Defaults to a function that checks if the string starts and ends with `{}` or `[]` - meaning anything that might be a JSON string, is attempted to be parsed - and if fails, proceeds.
+   *
+   * @param value - The string value to check.
+   * @param path - The JSON path leading to this value. e.g. `$[0].users[0].profile`
+   * @return `true` if the string should be JSON parsed.
+   */
+  shouldParse?: (value: string, path: string) => boolean
+
   /**
    * When `'in-place'`, arrays' and objects' values are parsed in-place. This is
    * the most time and space efficient option.
@@ -20,10 +31,22 @@ export interface ParseJSONResultsPluginOptions {
    * Defaults to `'in-place'`.
    */
   objectStrategy?: ObjectStrategy
+
+  /**
+   * The reviver function that will be passed to `JSON.parse`.
+   * See {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse#the_reviver_parameter | The reviver parameter}.
+   */
+  reviver?: (key: string, value: unknown, context?: any) => unknown
 }
 
 type ObjectStrategy = 'in-place' | 'create'
 
+type ProcessedParseJSONResultsPluginOptions = {
+  readonly [K in keyof ParseJSONResultsPluginOptions]-?: K extends 'skipKeys'
+    ? Record<string, true>
+    : ParseJSONResultsPluginOptions[K]
+}
+
 /**
  * Parses JSON strings in query results into JSON objects.
  *
@@ -69,10 +92,19 @@ type ObjectStrategy = 'in-place' | 'create'
  * ```
  */
 export class ParseJSONResultsPlugin implements KyselyPlugin {
-  readonly #objectStrategy: ObjectStrategy
-
-  constructor(readonly opt: ParseJSONResultsPluginOptions = {}) {
-    this.#objectStrategy = opt.objectStrategy || 'in-place'
+  readonly #options: ProcessedParseJSONResultsPluginOptions
+
+  constructor(readonly options: ParseJSONResultsPluginOptions = {}) {
+    const { shouldParse } = options
+
+    this.#options = freeze({
+      objectStrategy: options.objectStrategy || 'in-place',
+      reviver: options.reviver || ((_, value) => value),
+      shouldParse: shouldParse
+        ? (value: string, path: string) =>
+            maybeJson(value) && shouldParse(value, path)
+        : maybeJson,
+    })
   }
 
   // noop
@@ -85,61 +117,126 @@ export class ParseJSONResultsPlugin implements KyselyPlugin {
   ): Promise<QueryResult<UnknownRow>> {
     return {
       ...args.result,
-      rows: parseArray(args.result.rows, this.#objectStrategy),
+      rows: parseArray(args.result.rows, '$', this.#options),
     }
   }
 }
 
-function parseArray<T>(arr: T[], objectStrategy: ObjectStrategy): T[] {
-  const target = objectStrategy === 'create' ? new Array(arr.length) : arr
+function parseArray<T>(
+  arr: T[],
+  path: string,
+  options: ProcessedParseJSONResultsPluginOptions,
+): T[] {
+  const target =
+    options.objectStrategy === 'create' ? new Array(arr.length) : arr
 
   for (let i = 0; i < arr.length; ++i) {
-    target[i] = parse(arr[i], objectStrategy) as T
+    target[i] = parse(arr[i], `${path}[${i}]`, options) as T
   }
 
   return target
 }
 
-function parse(obj: unknown, objectStrategy: ObjectStrategy): unknown {
-  if (isString(obj)) {
-    return parseString(obj)
+function parse(
+  value: unknown,
+  path: string,
+  options: ProcessedParseJSONResultsPluginOptions,
+): unknown {
+  if (isString(value)) {
+    return parseString(value, path, options)
   }
 
-  if (Array.isArray(obj)) {
-    return parseArray(obj, objectStrategy)
+  if (Array.isArray(value)) {
+    return parseArray(value, path, options)
   }
 
-  if (isPlainObject(obj)) {
-    return parseObject(obj, objectStrategy)
+  if (isPlainObject(value)) {
+    return parseObject(value, path, options)
   }
 
-  return obj
+  return value
 }
 
-function parseString(str: string): unknown {
-  if (maybeJson(str)) {
-    try {
-      return parse(JSON.parse(str), 'in-place')
-    } catch (err) {
-      // this catch block is intentionally empty.
-    }
+function parseString(
+  str: string,
+  path: string,
+  options: ProcessedParseJSONResultsPluginOptions,
+): unknown {
+  const { shouldParse } = options
+
+  if (!shouldParse(str, path)) {
+    return str
   }
 
-  return str
+  try {
+    return parse(
+      JSON.parse(str, (key, value, ...otherArgs) => {
+        // prevent prototype pollution
+        if (key === '__proto__') {
+          return
+        }
+
+        // prevent prototype pollution
+        if (
+          key === 'constructor' &&
+          isPlainObject(value) &&
+          Object.hasOwn(value, 'prototype')
+        ) {
+          delete value.prototype
+        }
+
+        return options.reviver(key, value, ...otherArgs)
+      }),
+      path,
+      { ...options, objectStrategy: 'in-place' },
+    )
+  } catch (error) {
+    // custom JSON detection should expose parsing errors.
+    if (shouldParse !== maybeJson) {
+      throw error
+    }
+
+    // built-in naive heuristic should keep going despite errors given there might be false positives in detection.
+    console.error(error)
+
+    return str
+  }
 }
 
 function maybeJson(value: string): boolean {
-  return value.match(/^[\[\{]/) != null
+  return (
+    (value.startsWith('{') && value.endsWith('}')) ||
+    (value.startsWith('[') && value.endsWith(']'))
+  )
 }
 
 function parseObject(
   obj: Record<string, unknown>,
-  objectStrategy: ObjectStrategy,
+  path: string,
+  options: ProcessedParseJSONResultsPluginOptions,
 ): Record<string, unknown> {
+  const { objectStrategy } = options
+
   const target = objectStrategy === 'create' ? {} : obj
 
-  for (const key in obj) {
-    target[key] = parse(obj[key], objectStrategy)
+  for (const key of Object.keys(obj)) {
+    // prevent prototype pollution
+    if (key === '__proto__') {
+      continue
+    }
+
+    const parsed = parse(obj[key], `${path}.${key}`, options)
+
+    // prevent prototype pollution
+    if (
+      key === 'constructor' &&
+      isPlainObject(parsed) &&
+      Object.hasOwn(parsed, 'prototype')
+    ) {
+      delete parsed.prototype
+    }
+
+    target[key] = parsed
   }
 
   return target

src/plugin/parse-json-results/parse-json-results-plugin.ts

@@ -1,5 +1,7 @@
 import { ParseJSONResultsPlugin } from '../../..'
 import { createQueryId } from '../../../dist/cjs/util/query-id.js'
+import { expect } from './test-setup'
+import merge from 'prototype-pollution-vulnerable-lodash.merge-dont-upgrade'
 
 describe('ParseJSONResultsPlugin', () => {
   describe("when `objectStrategy` is 'create'", () => {
@@ -24,4 +26,47 @@ describe('ParseJSONResultsPlugin', () => {
       })
     })
   })
+
+  it('should omit dangerious keys when JSON parsing, denying prototype pollution downstream', async () => {
+    const plugin = new ParseJSONResultsPlugin({ objectStrategy: 'create' })
+
+    const maliciousRow = {
+      id: 1,
+      __proto__: JSON.stringify({ isAdmin: true }),
+      joe: JSON.stringify({
+        age: 30,
+        __proto__: { isAdmin: true },
+        constructor: JSON.stringify({
+          prototype: { isAdmin: true },
+          true: false,
+        }),
+        joe: JSON.stringify({
+          __proto__: { isAdmin: true },
+          true: false,
+        }),
+      }),
+      constructor: JSON.stringify({
+        prototype: { isAdmin: true },
+        true: false,
+        __proto__: { isAdmin: true },
+      }),
+      prototype: JSON.stringify({
+        isAdmin: true,
+        __proto__: { isAdmin: true },
+      }),
+    }
+
+    const {
+      rows: [rowParsedByPlugin],
+    } = await plugin.transformResult({
+      queryId: createQueryId(),
+      result: {
+        rows: [maliciousRow],
+      },
+    })
+
+    const mergedWithRowParsedByPlugin = merge({}, rowParsedByPlugin)
+
+    expect((mergedWithRowParsedByPlugin as any).isAdmin).to.be.undefined
+  })
 })