fix(item-service): block prototype-pollution via deep[] query aliases by khuepm · Pull Request #280 · khuepm/LumiBase

khuepm · 2026-07-17T09:14:52Z

Vấn đề

parseDeepQueryParams trong apps/cms/src/services/item-service.ts lấy alias trực tiếp từ query param người dùng (?deep[<alias>][fields|limit]=...) rồi dùng làm object key:

const current = deep[alias] ?? {};   // deep['__proto__'] -> Object.prototype
current.fields = value.split(',')...  // ghi vào Object.prototype
deep[alias] = current;

Với alias='__proto__', deep['__proto__'] ?? {} trả về chính Object.prototype → dòng gán current.fields ô nhiễm prototype toàn cục cho cả worker.

PoC (đã xác minh)

Request GET /...?deep[__proto__][fields]=x,y khiến ({}).fields === ['x','y'] — mọi object trong tiến trình bị nhiễm. Đây là lỗ hổng prototype pollution khai thác được, không phải lý thuyết.

Sửa

Thêm blocklist UNSAFE_ALIASES = { __proto__, constructor, prototype }.
Bỏ qua các alias này ở điểm ghi (parseDeepQueryParams) và điểm tiêu thụ (parseRelationFieldSelections, defense-in-depth). Không relation thật nào mang tên này.

Test

3 test mới trong item-relation-expansion.test.ts (chứng minh không còn pollute + vẫn giữ alias hợp lệ).

vitest run item-relation-expansion.test.ts → 13/13 pass
turbo run typecheck --filter=@lumibase/cms → pass

Setup impact

n/a — không chạm setup wizard, chỉ vá parsing query param.

parseDeepQueryParams read `deep[alias]` and wrote `deep[alias] = current` with `alias` taken straight from a user-supplied `deep[<alias>][field]` query param. For `alias='__proto__'`, `deep['__proto__'] ?? {}` resolves to Object.prototype, so `current.fields = [...]` polluted the global prototype for the whole worker (verified with a PoC: `?deep[__proto__][fields]=x` made `({}).fields === ['x']`). Add an UNSAFE_ALIASES blocklist (__proto__, constructor, prototype) and skip those aliases in both parseDeepQueryParams (the write sink) and parseRelationFieldSelections (defense-in-depth on the consumer). No real relation is ever named these. Covered by new prototype-pollution tests in item-relation-expansion.test.ts.

khuepm · 2026-07-18T20:19:14Z

Tôi sẽ soát codebase LumiBase để tìm các điểm rủi ro thực tế: hàm deep-merge, chỗ đọc option truyền vào spawn/exec, và endpoint nhận JSON body rồi merge sâu.

Kết quả audit đã có. Để tôi tóm tắt lại và xác nhận một điểm quan trọng: repo này thực tế là Next.js + Payload CMS, không phải Hono monorepo như CLAUDE.md mô tả — nên bối cảnh khác với giả định ban đầu.

Kết quả soát codebase

Đúng với dự đoán ở phần lý thuyết: có một hàm deep-merge tự viết không chặn key, và đây là điểm yếu duy nhất đáng kể.

Phát hiện chính — deepMerge.ts:18

export default function deepMerge<T, R>(target: T, source: R): T {
  const output = { ...target }
  if (isObject(target) && isObject(source)) {
    Object.keys(source).forEach((key) => {
      if (isObject(source[key])) {
        if (!(key in target)) {
          Object.assign(output, { [key]: source[key] })
        } else {
          output[key] = deepMerge(target[key], source[key])   // ← không lọc __proto__/constructor/prototype
        }
      } else {
        Object.assign(output, { [key]: source[key] })
      }
    })
  }
  return output
}

Đây chính là mẫu "guard hàm merge không có" mà comment nói tới. Nhưng — và đây là chỗ nối lại với ý kiến ban đầu — rủi ro hiện tại là latent (tiềm ẩn), không phải lỗ hổng sống:

Chỉ có 2 nơi gọi: link.ts:138 và linkGroup.ts:27.
Cả hai đều truyền overrides là config tĩnh do lập trình viên viết lúc định nghĩa schema, không phải request body.

Nói cách khác: điểm ghi (merge) tồn tại và không được bảo vệ, nhưng chưa có đường dẫn dữ liệu người dùng chạm tới nó. Nó sẽ thành lỗ hổng thực sự ngay khi ai đó dùng deepMerge với req.json().

Các mục còn lại — sạch

Hạng mục	Kết quả
Gadget child_process (spawn/exec đọc options.shell)	Không có trong mã ứng dụng
Endpoint merge JSON body	Không có (route đều do Payload sinh, delegate vào thư viện)
lodash.merge@4.6.2, deepmerge@4.3.1	Chỉ là transitive dep, không import trực tiếp; phiên bản đã ngoài dải CVE đã biết
Dynamic key obj[userKey]=v	Chỉ đúng tại deepMerge.ts:26 (cùng vấn đề trên)

Cái "gadget options.shell" mà comment lấy làm ví dụ kinh điển — codebase này không có. Đó là tin tốt: kể cả nếu pollution xảy ra, không có sink RCE rõ ràng để chuỗi tới.

Liên hệ lại với comment

Comment nói đúng về mặt cơ chế, và audit này xác nhận đúng cả cách phòng thủ nên làm: chặn tại điểm ghi (deepMerge) là fix rẻ nhất và bao phủ rộng nhất ở đây — vì bạn kiểm soát được hàm này, còn own-property check phải rải khắp mọi nơi đọc option.

Fix đề xuất (một dòng guard trong vòng lặp):

Object.keys(source).forEach((key) => {
  if (key === '__proto__' || key === 'constructor' || key === 'prototype') return
  // ... phần còn lại giữ nguyên
})

Hai lưu ý về độ chính xác (theo directive không khẳng định điều chưa kiểm chứng):

Trạng thái CVE của lodash.merge/deepmerge là [Inference] dựa trên số phiên bản — tôi chưa chạy pnpm audit để xác minh trực tiếp.
Khả năng khai thác của Object.keys với own-property "__proto__" phụ thuộc runtime — cần PoC để khẳng định chắc chắn, nhưng blocklist vẫn nên thêm bất kể.

Bạn muốn tôi thêm blocklist vào deepMerge.ts (kèm test nhỏ chứng minh nó chặn được payload {"__proto__":{"polluted":1}}), hay chạy pnpm audit để xác minh trạng thái CVE của hai transitive dep trước?

Tôi sẽ soát codebase LumiBase để tìm các điểm rủi ro thực tế: hàm deep-merge, chỗ đọc option truyền vào `spawn`/`exec`, và endpoint nhận JSON body rồi merge sâu.

Kết quả audit đã có. Để tôi tóm tắt lại và xác nhận một điểm quan trọng: repo này thực tế là Next.js + Payload CMS, không phải Hono monorepo như CLAUDE.md mô tả — nên bối cảnh khác với giả định ban đầu.

Kết quả soát codebase

Đúng với dự đoán ở phần lý thuyết: có một hàm deep-merge tự viết không chặn key, và đây là điểm yếu duy nhất đáng kể.

Phát hiện chính — [deepMerge.ts:18](src/utilities/deepMerge.ts:18)

export default function deepMerge<T, R>(target: T, source: R): T {
  const output = { ...target }
  if (isObject(target) && isObject(source)) {
    Object.keys(source).forEach((key) => {
      if (isObject(source[key])) {
        if (!(key in target)) {
          Object.assign(output, { [key]: source[key] })
        } else {
          output[key] = deepMerge(target[key], source[key])   // ← không lọc __proto__/constructor/prototype
        }
      } else {
        Object.assign(output, { [key]: source[key] })
      }
    })
  }
  return output
}

Đây chính là mẫu "guard hàm merge không có" mà comment nói tới. Nhưng — và đây là chỗ nối lại với ý kiến ban đầu — rủi ro hiện tại là latent (tiềm ẩn), không phải lỗ hổng sống:

Chỉ có 2 nơi gọi: [link.ts:138](src/fields/link.ts:138) và [linkGroup.ts:27](src/fields/linkGroup.ts:27).
Cả hai đều truyền overrides là config tĩnh do lập trình viên viết lúc định nghĩa schema, không phải request body.

Nói cách khác: điểm ghi (merge) tồn tại và không được bảo vệ, nhưng chưa có đường dẫn dữ liệu người dùng chạm tới nó. Nó sẽ thành lỗ hổng thực sự ngay khi ai đó dùng deepMerge với req.json().

Các mục còn lại — sạch

Hạng mục	Kết quả
Gadget `child_process` (`spawn`/`exec` đọc `options.shell`)	Không có trong mã ứng dụng
Endpoint merge JSON body	Không có (route đều do Payload sinh, delegate vào thư viện)
`lodash.merge@4.6.2`, `deepmerge@4.3.1`	Chỉ là transitive dep, không import trực tiếp; phiên bản đã ngoài dải CVE đã biết
Dynamic key `obj[userKey]=v`	Chỉ đúng tại `deepMerge.ts:26` (cùng vấn đề trên)

Cái "gadget options.shell" mà comment lấy làm ví dụ kinh điển — codebase này không có. Đó là tin tốt: kể cả nếu pollution xảy ra, không có sink RCE rõ ràng để chuỗi tới.

Liên hệ lại với comment

Comment nói đúng về mặt cơ chế, và audit này xác nhận đúng cả cách phòng thủ nên làm: chặn tại điểm ghi (deepMerge) là fix rẻ nhất và bao phủ rộng nhất ở đây — vì bạn kiểm soát được hàm này, còn own-property check phải rải khắp mọi nơi đọc option.

Fix đề xuất (một dòng guard trong vòng lặp):

Object.keys(source).forEach((key) => {
  if (key === '__proto__' || key === 'constructor' || key === 'prototype') return
  // ... phần còn lại giữ nguyên
})

Hai lưu ý về độ chính xác (theo directive không khẳng định điều chưa kiểm chứng):

Trạng thái CVE của lodash.merge/deepmerge là [Inference] dựa trên số phiên bản — tôi chưa chạy pnpm audit để xác minh trực tiếp.
Khả năng khai thác của Object.keys với own-property "__proto__" phụ thuộc runtime — cần PoC để khẳng định chắc chắn, nhưng blocklist vẫn nên thêm bất kể.

Bạn muốn tôi thêm blocklist vào deepMerge.ts (kèm test nhỏ chứng minh nó chặn được payload {"__proto__":{"polluted":1}}), hay chạy pnpm audit để xác minh trạng thái CVE của hai transitive dep trước?

khuepm · 2026-07-18T20:19:35Z

Tôi sẽ tiến hành audit đầy đủ: xác minh CVE bằng pnpm audit, xác nhận lỗ hổng deepMerge bằng PoC thực tế, rồi báo cáo. Bắt đầu song song các bước độc lập.

Kết quả PoC quan trọng: cả 3 test đều KHÔNG pollute được. Đây là phát hiện ngược với giả định của subagent — tôi cần xác minh lý do trước khi báo cáo sai. Để tôi kiểm tra sâu hơn tại sao.

Bây giờ tôi hiểu rõ cơ chế. Kết quả rõ ràng và cần một xác minh cuối để loại trừ mọi vector.

Audit hoàn tất. Kết quả xác minh bằng PoC thực tế lật ngược một phần kết luận của lần soát trước — và đây là lý do tôi phải chạy PoC thay vì tin vào đọc code.

Kết quả Audit — có xác minh bằng PoC

`deepMerge` KHÔNG bị prototype pollution — đã chứng minh bằng runtime

Tôi đã chạy 4 vector tấn công thực tế lên đúng hàm này. Tất cả đều thất bại — Object.prototype không bị nhiễm trong bất kỳ trường hợp nào.

Lý do (đã trace từng bước, không phải suy đoán):

Nhánh dòng 24 — Object.assign(output, { ['__proto__']: {...} }): gán qua computed key '__proto__' trong object literal. JS engine xử lý cái này như đặt lại prototype của output, không tạo own-property, và không ghi vào Object.prototype toàn cục. PoC: ({}).X === undefined sau khi gán.
Nhánh dòng 26 — output[key] = deepMerge(...) với key='__proto__' tính động (bracket notation): cũng chỉ set prototype của riêng output, không leo lên global prototype. PoC: ({}).Z === undefined.
Với payload {"__proto__":{...}}, '__proto__' in {} luôn true (inherited) → luôn rơi vào nhánh 26, và nhánh này vẫn an toàn.

Nói cách khác: hàm này tình cờ an toàn vì nó luôn thao tác trên output (một object mới từ spread { ...target }) qua các phép gán mà JS không cho phép leo global prototype — chứ không phải nhờ có blocklist.

So với báo cáo lần trước

Lần soát trước đánh giá đây là "Cao (latent)". PoC cho thấy cả rủi ro latent cũng gần như không có: kể cả khi bạn nối deepMerge với req.json() do người dùng kiểm soát, các vector __proto__/constructor.prototype chuẩn vẫn không pollute được. Đây là điểm mà chỉ đọc code sẽ kết luận sai.

`pnpm audit` — endpoint đã bị retire

ERR_PNPM_AUDIT_BAD_RESPONSE ... responded with 410: This endpoint is being retired.

pnpm bản này gọi endpoint audit cũ của npm đã ngừng hoạt động → tôi không xác minh được trạng thái CVE tự động. Trạng thái của lodash.merge@4.6.2 / deepmerge@4.3.1 vẫn là [Unverified].

Bảng tổng kết

Hạng mục	Kết quả	Bằng chứng
deepMerge prototype pollution	KHÔNG bị (4/4 PoC thất bại)	Runtime, trace từng nhánh
Gadget child_process	Không có trong mã app	grep sạch
Endpoint merge JSON body	Không có (route delegate vào Payload)	—
Dynamic key obj[userKey]=v	Chỉ tại deepMerge.ts:26, đã chứng minh an toàn	PoC
lodash.merge/deepmerge CVE	[Unverified] — transitive dep, không import trực tiếp	audit endpoint 410

Kết luận liên hệ với comment ban đầu

Comment đúng về mặt lý thuyết, và audit này minh họa đúng luận điểm cốt lõi của họ: "guard hàm merge không cứu được bạn" — vì cái cứu bạn ở đây không phải guard, mà là không có sink nào để pollution chuyển thành thiệt hại. Codebase này:

Không có gadget kiểu options.shell → dù có pollution cũng không leo tới RCE.
Bản thân deepMerge lại tình cờ không pollute được.

Hai việc còn treo, bạn muốn tôi làm không:

Vẫn thêm blocklist vào deepMerge như defense-in-depth (rẻ, và bảo vệ nếu ai đó refactor hàm sang dùng recursive assign kiểu khác trong tương lai) — dù hiện tại không cần thiết về mặt an ninh.
Xác minh CVE của 2 transitive dep bằng cách khác (osv-scanner, hoặc query trực tiếp OSV API) vì pnpm audit đã chết.

khuepm · 2026-07-18T20:19:58Z

Tôi sẽ thực hiện cả hai: thêm blocklist vào deepMerge (kèm test) và xác minh CVE qua OSV API.

Có thêm @corex/deepmerge@4.0.43. Tôi query OSV API cho cả 3 package và song song thêm blocklist + test.

CVE đã xác minh: cả 3 transitive dep đều 0 lỗ hổng theo OSV database. Giờ thêm blocklist defense-in-depth vào deepMerge và viết test chứng minh.

Giờ kiểm tra cách repo chạy test để viết test đúng convention.

Repo dùng vitest. Xem config để biết pattern test được include.

Test pattern là tests/int/**/*.int.spec.ts. Xem test hiện có để match convention rồi viết test cho deepMerge.

node_modules chưa được cài (npx kéo vitest tạm và không resolve được deps local). Kiểm tra xem đã cài chưa.

Worktree này chưa cài dependencies. Việc pnpm install đầy đủ (kéo Payload, Next, Playwright...) khá nặng. Vì test này thuần logic không cần Payload, tôi xác minh trực tiếp qua Node bằng cách transpile file thật — chứng minh chính đoạn code đã sửa hoạt động, không phải bản copy.

esbuild không có sẵn. File chỉ dùng TS type syntax đơn giản (generic, type guard) — Node 26 chạy TS trực tiếp được nếu tôi strip. Cách chắc chắn nhất: strip type annotations thủ công thành .mjs giữ nguyên logic, rồi chạy.

Toàn bộ 5 kiểm tra pass, chạy trên chính file deepMerge.ts đã sửa (chỉ đổi đuôi để Node type-strip, không đổi nội dung). Test dangerous key dropped, safe kept chứng minh blocklist mới thực sự loại __proto__ mà vẫn giữ key thường.

Dọn file tạm và xác nhận trạng thái cuối.

Audit hoàn tất. Tổng kết những gì đã thực hiện:

Kết quả Audit — đã thực thi đầy đủ

1. Xác minh CVE qua OSV API (thay `pnpm audit` đã chết)

Query trực tiếp api.osv.dev cho cả 3 transitive dep tìm thấy trong lockfile:

Package	Kết quả OSV
lodash.merge@4.6.2	0 lỗ hổng
deepmerge@4.3.1	0 lỗ hổng
@corex/deepmerge@4.0.43	0 lỗ hổng

Trạng thái CVE giờ đã verified (không còn [Unverified]). Cả 3 đều chỉ là transitive dep, không import trực tiếp trong src/.

2. Thêm blocklist defense-in-depth vào `deepMerge`

deepMerge.ts:13-27 — thêm PROTO_POLLUTION_KEYS và một guard sớm trong vòng lặp:

const PROTO_POLLUTION_KEYS = new Set(['__proto__', 'constructor', 'prototype'])
// ...
Object.keys(source).forEach((key) => {
  if (PROTO_POLLUTION_KEYS.has(key)) {
    return
  }
  // ...
})

Comment giải thích rõ đây là defense-in-depth: hàm vốn đã an toàn (đã chứng minh ở turn trước), guard này bảo vệ nếu tương lai có ai refactor sang recursive-assign kiểu khác hoặc feed dữ liệu request vào.

3. Test chứng minh — deepMerge.int.spec.ts

Viết test theo đúng convention repo (vitest, pattern tests/int/**/*.int.spec.ts). Vì worktree chưa có node_modules (chưa pnpm install), tôi chạy trực tiếp chính file deepMerge.ts đã sửa qua Node type-stripping — 5/5 pass, gồm cả test khẳng định blocklist loại __proto__ nhưng vẫn giữ key thường (safe: 1).

Lưu ý trung thực về trạng thái

Test file đã viết nhưng chưa chạy qua vitest runner của repo — vì worktree chưa cài dependencies. Tôi đã xác minh logic tương đương bằng cách chạy chính file nguồn qua Node. Khi bạn pnpm install xong, chạy pnpm test:int để xác nhận trong runner thật.
Chưa chạy typecheck/lint — cùng lý do (thiếu deps). File có // @ts-nocheck sẵn ở đầu nên thay đổi của tôi không tạo lỗi type.

Thay đổi: 1 file sửa (deepMerge.ts, +8 dòng), 1 file test mới. Chưa commit — bạn muốn tôi commit không, và có muốn tôi chạy full pnpm install + pnpm test:int để xác nhận trong runner thật trước khi commit không?

Tôi sẽ thực hiện cả hai: thêm blocklist vào `deepMerge` (kèm test) và xác minh CVE qua OSV API.