Switch obj.hasOwnProperty() to Object.hasOwn() where the receiver is untrusted
まだ誰も着手していません。
評価
- 難易度
- 2/5
- 見積もり時間
- 1〜3時間
- 初心者へのやさしさ
- 82/100
- issue の種類
- バグ
- 明瞭さ
- 明確に書かれている
- 活発さ
- 活発
- 技術スタック
- javascript, node.js
調査の方向性
utils.js、controllers/patchUpdate.js、controllers/patchUnset.js、controllers/patchSet.js に記載されている8つの呼び出し箇所から始め、controllers/crud.js:41 と controllers/bulk.js:73 から create および bulkCreate のパスを追跡します。receiver ベースのチェックを置き換え、通常のデータが引き続き機能する一方で、__rerum.hasOwnProperty を含むリクエストボディが 500 を発生させなくなることを確認します。
索引モデルが issue の本文から書いたものです。
説明
Summary
Eight call sites invoke hasOwnProperty as a method on the object being tested. RERUM stores arbitrary JSON, so an object under test can carry its own hasOwnProperty property, which shadows Object.prototype.hasOwnProperty and makes the call fail.
Why Object.hasOwn is immune
It is about which object is the receiver of the method lookup, not the method name.
obj.hasOwnProperty(k)resolveshasOwnPropertyonobj— own property first, then up the prototype chain. An own key namedhasOwnPropertyshadows the built-in.Object.hasOwn(obj, k)resolveshasOwnon theObjectconstructor.objis an ordinary argument and is never consulted to find the function.
--- obj.hasOwnProperty("__deleted") (receiver is the record) ---
plain record -> true
record w/ hasOwnProperty -> THROWS: TypeError: obj.hasOwnProperty is not a function
record w/ hasOwn -> true
record w/ fn override -> false <-- silently wrong
--- Object.hasOwn(obj, "__deleted") (receiver is Object) ---
plain record -> true
record w/ hasOwnProperty -> true
record w/ hasOwn -> true
record w/ fn override -> true
The failure mode depends on the value. A string throws a TypeError. A function returning false would not throw at all — a deleted record would be reported as live. That second case is unreachable here, because RERUM stores JSON and JSON cannot encode a function, so the realistic worst case is a 500.
Call sites
| # | Site | Receiver | Provenance | Verdict |
|---|---|---|---|---|
| 1 | utils.js:46 |
received_options |
request body | switch — highest priority |
| 2 | utils.js:72 |
received_options |
request body | switch — highest priority |
| 3 | utils.js:105 (isDeleted) |
obj |
stored record | switch |
| 4 | utils.js:113 (isReleased) |
obj |
stored record | switch |
| 5 | utils.js:114 (isReleased) |
obj.__rerum |
RERUM-minted, fixed shape | consistency only |
| 6 | controllers/patchUpdate.js:59 |
originalObject |
stored record | switch |
| 7 | controllers/patchUnset.js:65 |
originalObject |
stored record | switch |
| 8 | controllers/patchSet.js:63 |
originalObject |
stored record | switch |
Sites 1 and 2 are a different class from the rest
Sites 3–8 require a poisoned record to already exist in the database. Sites 1 and 2 take their receiver straight from the request:
--- utils.js:46/72 configureRerumOptions ---
OK normal create body
BREAKS body w/ __rerum.hasOwnProperty -> TypeError: received_options.hasOwnProperty is not a function
create() does provided = structuredClone(req.body), and configureRerumOptions() clones provided.__rerum into received_options. structuredClone preserves an own hasOwnProperty key, so POST /v1/api/create with a body of {"__rerum":{"hasOwnProperty":"x"}} throws. The call is at controllers/crud.js:41, before the try at line 54, so it escapes as a 500 rather than a 400. bulkCreate has the same shape through controllers/bulk.js:73.
Both require a valid token, so this is an authenticated self-inflicted 500 rather than an anonymous availability problem. It is still a 500 on well-formed JSON, which is the wrong answer to give.
putUpdate.js:115 is not affected — it passes extUpdate=true, and that branch sets received_options = {} before either check runs.
Site 5 is the only genuine non-issue
configureRerumOptions() builds rerumOptions from scratch and never spreads caller keys into it, so a stored __rerum always has the fixed RERUM shape. obj.__rerum.hasOwnProperty(...) only breaks if isReleased is handed a hand-made object, and all twelve callers of isDeleted/isReleased pass a record from db.findOne. Worth switching with the others so the next reader does not have to re-derive that it is safe.
Current impact: none
Measured against production (annotationStore.alpha):
'hasOwnProperty' present on stored records: 0
'__rerum.hasOwnProperty' present on stored records: 0
'__rerum.history.hasOwnProperty' present on stored records: 0
'__rerum.releases.hasOwnProperty' present on stored records: 0
Also checked and absent at the top level: constructor, __proto__, toString, valueOf, isPrototypeOf, propertyIsEnumerable.
Nothing stored today triggers any of this. Sites 1 and 2 do not depend on stored data, so those are reachable now by any client holding a token.
Suggested fix
Replace each site with Object.hasOwn(receiver, key):
// utils.js
const isDeleted = function(obj){
return Object.hasOwn(obj, "__deleted")
}
const isReleased = function(obj){
let bool =
(Object.hasOwn(obj, "__rerum") &&
Object.hasOwn(obj.__rerum, "isReleased") &&
obj.__rerum.isReleased !== "")
return bool
}
Eight lines across utils.js, controllers/patchUpdate.js, controllers/patchUnset.js, and controllers/patchSet.js. No behavior change for valid data. Object.hasOwn is available from Node 16.9, well under the repo's >=24.14.0 engine floor.
The older equivalent is Object.prototype.hasOwnProperty.call(obj, k), immune for the same reason.
- 主要言語
- JavaScript
- スター
- 3
- フォーク
- 6
- 平均マージ
- 4日 9時間
- マージ済み PR(30日)
- 5
環境構築
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
CenterForDigitalHumanities/rerum_server_nodejs のほかの issue
-
bug documentation
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
-
backend dependencies easy
難易度 2/5 1〜3時間 初心者へのやさしさ 84/100
-
難易度 4/5 3〜5日 初心者へのやさしさ 50/100
-
難易度 3/5 1〜2日 初心者へのやさしさ 74/100
-
Code Cleanup Epicオープン
難易度 4/5 3〜5日 初心者へのやさしさ 35/100
CenterForDigitalHumanities/rerum_server_nodejs の issue をすべて見る
似ている issue
-
factory-active factory-automatic harness/codex task-bug-reproduction-success task-identify-harness-labels-done task-identify-issue-type-done
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
メンテナーはふだん 1 日以内に返信
-
ux
難易度 1/5 1時間未満 初心者へのやさしさ 90/100
rr-djk/rr-djuikoo.com#53 ·
メンテナーはふだん 1 日以内に返信
-
new spec review
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
w3c/browser-specs#2666 · コメント 1 件 ·
メンテナーはふだん 3 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
thim81/openapi-format#238 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
decentespresso/dye2#13 ·