[3] GetByStatus walked the prototype chain for direct property access

HighJSC bytecode profiling and DFG JITTypeConfusion

A private field read that answers with the prototype's value

b07dca9

High. 하나의 분석 루틴이 두 가지 연산을 동시에 처리하고 있었는데, 이 둘의 실질적인 차이는 lookup이 어디까지 탐색을 허용하는지뿐이었습니다. 이후 변경이 이 루틴에 더 넓은 범위를 탐색하도록 가르치면서 문제가 생겼습니다. 결과적으로 컴파일된 코드가, 언어 스펙상 miss가 보장되는 상황에서도 own-property hit으로 보고할 수 있게 됩니다 — brand-check bypass에 해당합니다. Memory-safety 확장 여부는 이 check을 layout의 증거로 신뢰하는 consumer가 있는지에 달려 있습니다.

JavaScript의 property load에는 두 가지 방식이 있습니다. 하나는 object를 조회한 뒤 prototype chain을 따라가는 일반 lookup이고, 다른 하나는 object 자신에서 멈춰야 하는 own-property-only lookup입니다. GetByStatus는 property-get bytecode가 수행하는 동작을 컴파일러가 요약한 것으로, State와 어떤 base shape에 적용되는지·값이 어디에 있는지를 설명하는 GetByVariant 목록으로 구성됩니다. DFG의 abstract interpreter와 constant-folding phase는 모두 이 정보를 참조해 access를 inline하거나 고정 offset으로 fold할 수 있는지 판단합니다. 이 phase들이 지켜야 하는 soundness 규칙은, optimizing tier가 어떤 연산을 재작성할 때 명세된 동작과 관찰상 동등한 결과로만 바꿀 수 있다는 것입니다.

관전 포인트: class private-field read와 self-hosted builtin의 own-property probe가, 언어 스펙상 undefined나 TypeError가 보장되는 상황에서도 DFG-compiled code에서 prototype의 값을 반환하도록 만들 수 있습니다. 일반 script에서 도달 가능한 brand-check bypass입니다.

When computing the GetByStatus, we should check if the property lookup is a direct property access before doing a prototype walk since direct accesses are not supposed to consult the prototype. Originally landed as 305413.572@rapid/safari-7624.2.5.110-branch.

Source/JavaScriptCore/bytecode/GetByStatus.cpp

-GetByStatus GetByStatus::computeFor(JSGlobalObject* globalObject, const StructureSet& set, CacheableIdentifier identifier)
+GetByStatus GetByStatus::computeFor(JSGlobalObject* globalObject, const StructureSet& set, CacheableIdentifier identifier, GetByStatus::LookupMode mode)
 {
     // For now we only handle the super simple self access case. We could handle the
     // prototype case in the future.
 
-    //
 
-    // Note that this code is also used for GetByIdDirect since this function only looks
 
-    // into direct properties. When supporting prototype chains, we should split this for
 
-    // GetById and GetByIdDirect.
 ...
 
-    if (auto result = attempToFold())
 
-        return result.value();
+    // We should do a prototype walk searching for the property only if this
+    // is a normal access. Don't consult proto for for direct accesses.
+    if (mode == GetByStatus::LookupMode::Normal) {
+        if (auto result = attempToFold())
+            return result.value();
+    }

Source/JavaScriptCore/dfg/DFGNode.h

+    GetByStatus::LookupMode propertyLookupMode()
+    {
+        switch (op()) {
+        case GetByIdDirect:
+        case GetByIdDirectFlush:
+        case GetPrivateNameById:
+            return GetByStatus::LookupMode::Direct;
+        case GetById:
+        case GetByIdFlush:
+        case GetByIdMegamorphic:
+            return GetByStatus::LookupMode::Normal;
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+        }
+    }

Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

 
-                GetByStatus status = GetByStatus::computeFor(m_graph.globalObjectFor(node->origin.semantic), baseValue.m_structure.toStructureSet(), identifier);
+                GetByStatus::LookupMode lookupMode = node->propertyLookupMode();
+                GetByStatus status = GetByStatus::computeFor(m_graph.globalObjectFor(node->origin.semantic), baseValue.m_structure.toStructureSet(), identifier, lookupMode);

Patch Details

이번 변경은 GetByStatus::computeFor의 structure-set overload에 새로운 lookup-scope discriminator를 추가로 넘겨주고, DFG node 스스로가 자신의 opcode가 어떤 scope를 의미하는지 보고하도록 만들었습니다. 그리고 세 곳의 call site가 이 값을 전달하도록 수정되었습니다.

GetByStatus.h에는 enum class LookupMode : bool { Normal, Direct }가 추가되었고, overload는 computeFor(JSGlobalObject*, const StructureSet&, CacheableIdentifier, LookupMode)로 바뀌었습니다. GetByStatus.cpp에서는 기존에 무조건 실행되던 if (auto result = attempToFold()) return result.value(); 부분이 mode == LookupMode::Normal 조건 아래로 들어갔습니다. 또한 "이 루틴은 direct property만 조회한다"는 잘못된 내용을 담고 있던 기존 주석과, prototype chain을 지원하게 되면 GetById와 GetByIdDirect를 분리해야 한다는 TODO도 함께 삭제되었습니다.

DFGNode.h에는 Node::propertyLookupMode()가 추가되어, GetByIdDirect·GetByIdDirectFlush·GetPrivateNameById는 LookupMode::Direct로, GetById·GetByIdFlush·GetByIdMegamorphic는 LookupMode::Normal로 매핑됩니다. 그 외 케이스는 RELEASE_ASSERT_NOT_REACHED()로 처리됩니다. DFG의 두 consumer인 AbstractInterpreter<>::executeEffects와 ConstantFoldingPhase::foldConstants는 이제 node->propertyLookupMode()를 전달하며, ByteCodeParser::parseBlock의 op_get_from_scope global-property 경로는 LookupMode::Normal을 명시적으로 전달합니다. 이 경로에는 global property lookup이 원래 global object의 prototype chain을 조회하도록 되어 있다는 주석이 함께 달려 있습니다.

하나의 lookup 루틴을 서로 다른 lookup scope를 가진 두 연산이 공유하면서, prototype-chain 탐색이 own-property-only access에 조용히 새어 들어간 사례입니다.

Background

이 코드가 있는 위치. GetByStatus는 JSC가 property get을 bytecode 수준에서 요약한 정보로, State(Simple, Megamorphic, LikelyTakesSlowPath 등)와 GetByVariant 목록으로 구성됩니다. 각 GetByVariant는 어떤 base shape에 적용되는지를 나타내는 StructureSet, object storage 내 값의 위치를 나타내는 PropertyOffset, 그리고 ObjectPropertyConditionSet을 갖습니다. GetByVariant.h에는 "A non-empty condition set means that this is a prototype load"라고 명시되어 있습니다.

Structure. Structure는 object의 shape를 기술하며 property name을 offset으로 매핑합니다. structure->getConcurrently(uid)는 compiler thread에서 structure의 own property offset을 조회하는 데 사용됩니다.

세 가지 get 연산. GetById는 object를 조회한 뒤 prototype chain을 따라가는 일반 JavaScript property lookup을 구현합니다. GetByIdDirect는 own-property-only load를 구현하며, JSC의 self-hosted builtin JavaScript가 prototype getter를 호출하지 않고 object 자신의 상태를 확인할 때 사용됩니다. GetPrivateNameById는 class private-field read(obj.#x)를 구현하는데, 언어 규칙상 해당 field가 바로 그 object에 설치되어 있을 때만 성공해야 합니다.

Abstract interpretation과 folding. DFG abstract interpreter(AbstractInterpreter<>::executeEffects)는 graph를 순회하며 각 node의 값에 대한 사실을 증명합니다. 어떤 사실이 증명되면 이후 phase가 runtime check를 생략할 수 있게 됩니다. ConstantFoldingPhase는 이 사실들을 소비해 node를 재작성하는데, Simple 상태에 variant가 하나뿐이라면 일반적인 get을 CheckStructure와 고정 offset GetByOffset의 조합으로, 또는 JSConstant로 대체할 수 있습니다.

op_get_from_scope. scope object에서 변수를 읽어오는 bytecode로, global-object property를 읽는 경우도 포함합니다.

Analysis

이 버그의 본질은 semantic mismatch입니다. optimizer가 own-property-only 연산을 prototype-chain lookup 의미론으로 모델링하고 있었습니다.

  child = Object.create(p)                Lookup scope per opcode
  ┌──────────────┐                        GetById            : child -> p -> ...
  │ child        │  own: (none)           GetByIdDirect      : child only
  │  [[Proto]] ──┼──┐                     GetPrivateNameById : child only
  └──────────────┘  │
                    v
             ┌──────────────┐
             │ p            │  own: #x @ offset 0
             └──────────────┘

  Pre-fix fold of a Direct-mode node over child's structure set:
    attempToFold() walks to p, returns Simple variant with a
    non-empty conditionSet and p's offset -> compiled code reads p.#x

GetByIdDirect / GetPrivateNameById node의 base가 finite structure set을 가진다고 증명된 상태(executeEffects의 value.m_structure.isFinite(), foldConstants의 baseValue.m_structure.toStructureSet())에서도, 그 property가 set 안의 모든 structure에서 부재할 수 있습니다. 이 상태에서 direct-access의 올바른 의미론은 GetByIdDirect라면 undefined, field가 없는 object에 대한 private-name load라면 TypeError여야 합니다. 하지만 패치 이전에는 attempToFold()가 그대로 실행되어 해당 structure들의 prototype chain을 탐색했습니다. prototype이 그 identifier를 갖고 있으면, computeFor는 prototype chain을 기술하는 variant와 prototype의 storage를 가리키는 offset을 담은 Simple status를 반환했습니다. 이후 ConstantFoldingPhase가 이 node를 structure check와 condition check로 보호된 prototype load로 재작성하고, abstract interpreter는 node의 결과를 prototype property가 증명하는 값으로 narrowing합니다.

하나의 root cause에서 두 가지 결과가 이어집니다. 첫째, 올바르게 guard된 input에 대해 컴파일된 코드가 잘못된 값을 계산합니다. 둘째, abstract interpreter가 연산의 실제 의미론으로는 나올 수 없는 result type을 단정하게 됩니다 — downstream type check를 생략시키는 전형적인 "AI lies" 전제 조건에 해당합니다.

attempToFold()가 반환하는 값의 정확한 형태는 직접 확인된 것이 아니라 정황을 통해 도출한 내용입니다. 추가된 주석은 이 함수가 prototype walk를 수행한다는 점을 확인해주고, GetByVariant.h는 non-empty condition set이 prototype load를 의미한다고 문서화하고 있습니다. 다만 attempToFold()의 본문 자체는 제공된 GetByStatus.cpp의 truncation 지점 너머에 있어 직접 확인되지는 않습니다.

두 node family 모두 web content에서 도달 가능합니다. GetPrivateNameById는 일반적인 class private-field read에서 emit되고, GetByIdDirect/GetByIdDirectFlush는 일반 JS가 호출하는 self-hosted builtin 내부에서 emit됩니다. 따라서 페이지가 hot loop를 통해 둘 중 어느 쪽이든 DFG tier까지 끌어올릴 수 있습니다. Trigger 형태는 다음과 같습니다.

class C { #x = 1; static read(o) { return o.#x; } }.
#x를 own property로 갖는 instance p를 만듭니다.
#x를 own property로 갖지 않는 look-alike child = Object.create(p)를 만듭니다.
C.read를 child의 abstract structure set을 가진 base에 대해 hot하게 실행하되, throw되는 케이스를 try/catch로 감싸 함수가 계속 tier up되도록 합니다.

패치 이전에는 foldConstants가 baseValue.m_structure.toStructureSet()으로 computeFor를 호출하고, attempToFold()가 child의 prototype chain을 탐색해 #x를 p에서 찾은 뒤 p의 offset을 가진 Simple variant를 반환합니다. DFG는 guard된 prototype load를 emit하므로, 컴파일된 C.read(child)가 interpreter에서는 TypeError가 발생하는 상황에서 1을 반환할 수 있었습니다. builtin 내부의 own-property probe도 같은 형태의 문제를 겪습니다. Object.create(realInstance)로 만든 look-alike의 own-property probe는 원래 undefined를 읽어야 하지만, DFG 하에서는 prototype의 값을 읽어버려 instance brand check로서의 probe 용도가 무력화될 수 있습니다.

logic bypass를 넘어선 확장 여부는, 이 bypass된 check을 layout의 증거로 신뢰하는 consumer가 존재하는지에 달려 있습니다. builtin이나 downstream JIT-visible 경로가 이 bogus probe 성공 이후 look-alike의 고정된 internal-field slot을 계속 건드린다면, type-confused access로 이어질 가능성이 있습니다. builtin의 소스 코드는 제공된 context에 포함되어 있지 않으므로, 이 단계는 예상되는 방향으로만 제시합니다. 이와는 별개로, abstract-interpreter 호출 지점을 기준으로 삼는 두 번째 확장 시나리오도 있습니다. executeEffects는 fold된 prototype variant로부터 node의 AbstractValue를 narrowing하므로, direct-access의 실제 의미론으로는 나올 수 없는 type을 증명해버릴 수 있습니다. 이 증명을 근거로 check이 생략된 downstream edge가 있다면, 잘못된 type의 값을 다루게 될 가능성이 있습니다.

이 vulnerability는 interpreter의 언어 의미론과 DFG-compiled code 사이의 soundness 경계를 약화시킵니다. own-property-only load가 prototype-chain load로 재작성될 수 있었기 때문에, property를 own하지 않는 object가 — 컴파일된 코드에서만 — 그 property를 own한 것처럼 보일 수 있습니다. 이 상태에 web content에서 도달한 공격자는, JavaScript 코드가 (JSC 자체의 self-hosted builtin을 포함해) 진짜 instance와 조작된 look-alike를 구분하기 위해 의존하는 own-property 및 private-field brand check을 우회할 수 있고, 언어가 접근 불가를 보장하는 private 또는 internal 상태를 읽어낼 수 있습니다. 이렇게 우회된 check의 downstream consumer가 특정 object layout을 가정하고 동작한다면, logic bypass에서 memory-safety primitive로 확장될 가능성이 있습니다.

삭제된 주석 자체가 이 문제의 배경을 말해줍니다. 원래 이 공유 루틴은 자신의 correctness precondition("이 함수는 direct property만 조회한다")을 문서화하고, 그에 대한 해법("GetById와 GetByIdDirect를 분리해야 한다")까지 명시하고 있었습니다. 이후 변경이 prototype을 탐색하는 attempToFold()를 같은 함수에 추가하면서, 주석은 그대로 둔 채 이 precondition을 깨뜨렸습니다. 이번 fix는 바로 그 TODO가 요구했던 분리를, 두 함수 대신 하나의 parameter로 구현한 것입니다. 그리고 Node::propertyLookupMode()의 RELEASE_ASSERT_NOT_REACHED() default는, 앞으로 이 경로를 타는 어떤 새로운 node type이라도 자신의 lookup 의미론을 명시적으로 선언하도록 강제합니다.

Audit directions

여러 bytecode 연산이 하나의 분석 루틴을 공유하지만 서로 lookup scope가 다른 경우. Source/JavaScriptCore/bytecode/에 있는 다른 *Status::computeFor 계열 — PutByStatus, InByStatus, DeleteByStatus, CheckPrivateBrandStatus, SetPrivateBrandStatus, InstanceOfStatus — 을 점검하여, 하나의 overload가 direct/private variant와 normal variant를 동시에 처리하고 있는지 확인할 필요가 있습니다. 좁은 단서는, DFGConstantFoldingPhase.cpp / DFGAbstractInterpreterInlines.h의 caller가 여러 NodeType을 하나의 case group으로 묶어 처리하면서도 이들을 구분하는 discriminator를 넘기지 않는 computeFor입니다. 조금 더 넓은 단서는, name과 container만 받고 scope flag(own vs inherited, enumerable vs all, string-keyed vs symbol-keyed)는 받지 않는 lookup helper 전반입니다. ObjectPropertyConditionSet builder와 ComplexGetStatus::computeFor가 이 형태를 살펴볼 지점입니다. 가장 넓게 보면, 이는 "implicit scope parameter를 가진 공유 resolver"라는 일반적인 클래스에 속하며, Python의 getattr vs __dict__ fast path, V8의 LookupIterator configuration mode, 혹은 own-column과 joined relation 사이에서 하나의 field resolver를 공유하는 ORM 등에도 동일하게 적용될 수 있습니다. 여기서 가져갈 invariant는, resolver가 caller가 속한 연산이 허용하는 범위보다 더 넓게 탐색할 수 있다면 그 탐색 범위는 반드시 명시적 parameter로 표현되어야지 주석으로만 남겨져서는 안 된다는 점입니다.

Bug hunting 관점에서 보면, 특히 세 가지 지점이 눈에 띕니다.

공유 코드에 대한 correctness precondition을 기록한 comment가, 이후 같은 함수에 가해진 무관한 변경으로 무효화되는 경우. Source/JavaScriptCore/bytecode/와 Source/JavaScriptCore/dfg/에서 "also used for", "only looks", "we should split", "when supporting" 문구가 포함된 comment를 검색하고, 그 아래 코드가 여전히 comment가 명시한 precondition을 만족하는지 확인할 필요가 있습니다. 좁게 보면, "this only handles X"처럼 함수의 속성을 단정하는 comment 아래에 실제로는 not-X까지 처리하는 body가 놓여 있는 경우가 tell입니다. 조금 더 넓게 보면, prose로만 문서화되고 코드로는 assert되지 않은 invariant를 다루는 클래스 전체가 해당됩니다 — split 이나 separate path를 언급하는 FIXME/TODO가 있으면서 실제로는 그 분리가 일어나지 않은 함수를 찾아보십시오. 바로 그 미래의 변경이 precondition을 깨뜨리는 지점이기 때문입니다. 가장 넓게 보면, comment에만 명시된 precondition은 강제되지 않는다는 원칙 자체가 핵심입니다. 따라서 "this is safe because…" 형태의 모든 comment는 assert되지 않은 invariant로 간주하고 현재 body와 대조해서 확인해야 합니다. 이 원리는 Linux kernel의 locking comment, Rust unsafe justification block, Go의 // caller must hold 관례에도 그대로 적용됩니다. 각 hit의 검증은 grep으로 끝나지 않고 수동으로 코드를 읽어야 합니다.
status summary로부터 도출된 abstract-interpretation fact가, 실제로 interpret되는 node와 다른 operation을 모델링하는 경우. DFGAbstractInterpreterInlines.h와 DFGConstantFoldingPhase.cpp에서 profiling data가 아니라 value.m_structure.toStructureSet()으로부터 *Status를 구성하는 모든 지점을 점검하고, status의 각 필드가 해당 node 고유의 opcode semantics로부터 도출되는지 확인해야 합니다. node->cacheableIdentifier()는 이미 그렇게 되어 있고, LookupMode도 이제는 그렇게 되어 있습니다. 다만 strictness, viaGlobalProxy, private-brand requirement 같은 암묵적 가정이 추가로 남아 있는지 확인해야 합니다. 좁게 보면, 여러 op를 포괄하는 switch (node->op()) arm 안에서 호출되는 computeFor인데, 그 인자에 node->op()에서 도출된 discriminator가 전혀 없는 경우가 tell입니다. 조금 더 넓게 보면, N개의 서로 다른 IR opcode를 하나의 공유 semantic model object로 매핑하는 모든 optimizer phase가 대상입니다 — 그 model이 각 opcode의 contract를 재구성할 만큼 충분한 discriminator를 갖고 있는지 점검해야 합니다. 가장 넓게 보면, 이는 "optimizer summary가 semantics의 operand 하나를 놓치는" 일반적인 버그 클래스에 해당하며, LLVM의 MemoryLocation/alias-analysis summary나 SpiderMonkey의 MIR alias set에도 그대로 전이됩니다. 여러 opcode에 걸쳐 공유되는 transfer function은 구분되는 모든 operand를 명시적으로 받아야 합니다. 그렇지 않으면 그 operand들 행동의 union을 계산하는 셈이 됩니다.
language-level own-property 및 private-field brand check에, prototype이 제공한 값을 관찰할 수 있는 다른 JIT 경로가 존재하는지 점검할 필요가 있습니다. 새로 추가된 Node::propertyLookupMode()에 열거된 GetByIdDirect, GetByIdDirectFlush, GetPrivateNameById node type에서 시작해서, 이를 소비하는 모든 DFG/FTL phase를 추적하고, 이어서 private-brand node (CheckPrivateBrand, SetPrivateBrand)와 그 *Status helper에 대해서도 동일하게 반복해야 합니다. 일치하는 tell은, Direct-mode node type 중 하나를 처리하면서 conditionSet()이 비어 있지 않은 GetByVariant에 도달하는 phase입니다. 이 fix 이후로는 그런 조합 자체가 구조적으로 불가능해지므로, 이런 조합이 발견된다면 살아있는 variant라는 의미입니다.