Просмотр исходного кода

fix(schema): harden realm-safe JSON validation

Tianyi Cui 2 месяцев назад
Родитель
Сommit
384cc8c157

+ 3 - 0
packages/cordis/tool-cordis/src/guard.ts

@@ -55,6 +55,9 @@ function cloneJson(value: unknown, path: string, seen = new Set<object>()): unkn
       return output
     }
     if (!isPlainRecord(value)) throw new Error(`harness.defineTool ${path} must be lossless JSON data`)
+    if (Reflect.ownKeys(value).some(key => typeof key !== 'string' || !Object.prototype.propertyIsEnumerable.call(value, key))) {
+      throw new Error(`harness.defineTool ${path} must be lossless JSON data`)
+    }
     const output: Record<string, unknown> = {}
     for (const [key, entry] of Object.entries(value)) {
       Object.defineProperty(output, key, {

+ 2 - 0
packages/cordis/tool-cordis/tests/mount.spec.ts

@@ -345,6 +345,8 @@ describe('cordis_mount', () => {
     ['parameters: { value: { type: \'json\', default: Array(2) } }', 'parameters.value.default must be lossless JSON data'],
     ['parameters: { value: { type: \'json\', default: Object.assign([1], { extra: true }) } }', 'parameters.value.default must be lossless JSON data'],
     ['parameters: { value: { type: \'json\', default: (() => { const v = Array(1); v.extra = true; return v })() } }', 'parameters.value.default must be lossless JSON data'],
+    ['parameters: { value: { type: \'json\', default: Object.defineProperty({}, \'hidden\', { value: true }) } }', 'parameters.value.default must be lossless JSON data'],
+    ['parameters: { value: { type: \'json\', default: { [Symbol(\'hidden\')]: true } } }', 'parameters.value.default must be lossless JSON data'],
     ['parameters: { value: { type: \'json\', default: new (class DefaultValue { constructor() { this.ok = true } })() } }', 'parameters.value.default must be lossless JSON data'],
     ['parameters: { value: { type: \'json\', default: new Date(0) } }', 'parameters.value.default must be lossless JSON data'],
   ])('rejects a malformed ParameterSchemaSpec (%s) with a teaching error', async (parameters, message) => {

+ 17 - 3
packages/core/session/src/json.ts

@@ -15,7 +15,11 @@ export type JsonValue = null | boolean | number | string | JsonValue[] | { [key:
 /** Whether an array uses one realm's intrinsic `Array.prototype`, not a subclass or forged prototype. */
 function hasPlainArrayPrototype(value: unknown[]): boolean {
   const prototype: unknown = Object.getPrototypeOf(value)
-  return Array.isArray(prototype) && Object.getPrototypeOf(Object.getPrototypeOf(prototype)) === null
+  if (!Array.isArray(prototype)) return false
+  const objectPrototype: unknown = Object.getPrototypeOf(prototype)
+  return objectPrototype !== null
+    && !Array.isArray(objectPrototype)
+    && Object.getPrototypeOf(objectPrototype) === null
 }
 
 /** Whether an object is a plain or null-prototype record from any JavaScript realm. */
@@ -24,6 +28,13 @@ function hasPlainObjectPrototype(value: object): boolean {
   return prototype === null || Object.getPrototypeOf(prototype) === null
 }
 
+/** Return every JSON-visible object key, or reject own data JSON would discard. */
+function enumerableStringKeys(value: object): string[] | undefined {
+  const keys = Reflect.ownKeys(value)
+  if (keys.some(key => typeof key !== 'string' || !Object.prototype.propertyIsEnumerable.call(value, key))) return undefined
+  return keys as string[]
+}
+
 /**
  * Validate and detach lossless JSON in one read per property, so a stateful
  * getter cannot change between validation and copying. Accepts ordinary arrays,
@@ -75,8 +86,10 @@ export function snapshotJsonValue<T>(value: T): T | undefined {
       }
 
       if (!hasPlainObjectPrototype(current)) return undefined
+      const keys = enumerableStringKeys(current)
+      if (keys === undefined) return undefined
       const snapshot: { [key: string]: JsonValue } = {}
-      for (const key of Object.keys(current)) {
+      for (const key of keys) {
         const item = visit((current as Record<string, unknown>)[key])
         if (item === undefined) return undefined
         // Define the key as data so a JSON field literally named "__proto__"
@@ -139,7 +152,8 @@ export function isJsonValue(value: unknown, seen: Set<object> = new Set()): bool
     }
     // Plain object only (reject Map/Set/Date/class instances).
     if (!hasPlainObjectPrototype(value)) return false
-    return Object.values(value).every(v => isJsonValue(v, seen))
+    const keys = enumerableStringKeys(value)
+    return keys !== undefined && keys.every(key => isJsonValue((value as Record<string, unknown>)[key], seen))
   } finally {
     seen.delete(value)
   }

+ 18 - 0
packages/core/session/tests/json.spec.ts

@@ -92,6 +92,12 @@ describe('snapshotJsonValue', () => {
     Object.defineProperty(decorated, 'extra', { value: true })
     const symbolDecorated = [1]
     Object.defineProperty(symbolDecorated, Symbol('extra'), { value: true })
+    const hiddenObject = Object.defineProperty({}, 'hidden', { value: true })
+    const symbolObject = { [Symbol('extra')]: true }
+    const forgedPrototype: unknown[] = []
+    Object.setPrototypeOf(forgedPrototype, null)
+    const forgedArray = [1]
+    Object.setPrototypeOf(forgedArray, forgedPrototype)
     const cyclic: Record<string, unknown> = {}
     cyclic.self = cyclic
     const foreignExotics = runInNewContext(`(() => {
@@ -109,6 +115,9 @@ describe('snapshotJsonValue', () => {
     expect(snapshotJsonValue(compensatedSparse)).toBeUndefined()
     expect(snapshotJsonValue(decorated)).toBeUndefined()
     expect(snapshotJsonValue(symbolDecorated)).toBeUndefined()
+    expect(snapshotJsonValue(hiddenObject)).toBeUndefined()
+    expect(snapshotJsonValue(symbolObject)).toBeUndefined()
+    expect(snapshotJsonValue(forgedArray)).toBeUndefined()
     expect(snapshotJsonValue(cyclic)).toBeUndefined()
     expect(snapshotJsonValue([undefined])).toBeUndefined()
     expect(snapshotJsonValue({ value: undefined })).toBeUndefined()
@@ -177,6 +186,12 @@ describe('isJsonValue', () => {
     const decorated = Object.assign([1], { extra: true })
     const symbolDecorated = [1]
     Object.defineProperty(symbolDecorated, Symbol('extra'), { value: true })
+    const hiddenObject = Object.defineProperty({}, 'hidden', { value: true })
+    const symbolObject = { [Symbol('extra')]: true }
+    const forgedPrototype: unknown[] = []
+    Object.setPrototypeOf(forgedPrototype, null)
+    const forgedArray = [1]
+    Object.setPrototypeOf(forgedArray, forgedPrototype)
     const cyclic: Record<string, unknown> = {}
     cyclic.self = cyclic
 
@@ -184,6 +199,9 @@ describe('isJsonValue', () => {
     expect(isJsonValue(compensatedSparse)).toBe(false)
     expect(isJsonValue(decorated)).toBe(false)
     expect(isJsonValue(symbolDecorated)).toBe(false)
+    expect(isJsonValue(hiddenObject)).toBe(false)
+    expect(isJsonValue(symbolObject)).toBe(false)
+    expect(isJsonValue(forgedArray)).toBe(false)
     expect(isJsonValue(new ExoticArray(1))).toBe(false)
     expect(isJsonValue([undefined])).toBe(false)
     expect(isJsonValue({ value: undefined })).toBe(false)

+ 1 - 1
packages/core/tools/src/json-schema.ts

@@ -351,7 +351,7 @@ function checkValueUnchecked(node: JsonSchemaNode, value: unknown, path: string)
       return safelyIsJsonValue(value) ? [] : [`"${diagnosticPath(path)}" must be a lossless JSON object`]
     }
     case 'array': {
-      if (!Array.isArray(value) || Object.getPrototypeOf(value) !== Array.prototype) return [`"${diagnosticPath(path)}" must be an array`]
+      if (!Array.isArray(value)) return [`"${diagnosticPath(path)}" must be an array`]
       const items = node.items
       const violations = items === undefined
         ? []

+ 5 - 0
packages/core/tools/tests/json-schema.spec.ts

@@ -186,6 +186,10 @@ describe('the enforced raw JSON Schema subset', () => {
     })
     expect(violationsOf({ examples: explosive }))
       .toEqual(['schema.examples annotation must be lossless JSON data'])
+    expect(violationsOf({ default: Object.defineProperty({}, 'hidden', { value: true }) }))
+      .toEqual(['schema.default annotation must be lossless JSON data'])
+    expect(violationsOf({ default: { [Symbol('hidden')]: true } }))
+      .toEqual(['schema.default annotation must be lossless JSON data'])
   })
 
   it('accepts lossless annotation containers from another JavaScript realm', () => {
@@ -298,6 +302,7 @@ describe('validateJsonSchemaValue', () => {
   it('validates dense arrays per index and rejects lossy arrays', () => {
     const schema = asserted({ type: 'array', items: { type: 'integer' } })
     expect(validateJsonSchemaValue(schema, [1, 2])).toEqual([])
+    expect(validateJsonSchemaValue(schema, runInNewContext('[1, 2]'))).toEqual([])
     expect(validateJsonSchemaValue(schema, [1, 1.5])).toEqual(['"value[1]" must be an integer'])
     expect(validateJsonSchemaValue(schema, 'x')).toEqual(['"value" must be an array'])
     const sparse: number[] = []