CVE-2024-27834

Overview

Advisory: Apple Security Advisory

Impact:

Description: An attacker with arbitrary read and write capability may be able to bypass Pointer Authentication

Researchers: Manfred Paul (@_manfp) working with Trend Micro’s Zero Day Initiative

Attribute Value
CVE CVE-2024-27834
Bugzilla 272750
Component JSC
Bug Class LogicError
Severity high
Commit 3e3d0883c8495547…
Advisory Apple Advisory

Root Cause Analysis

The issue was addressed with improved checks.

Files Changed

Source Files

  • Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h
  • Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
  • Source/JavaScriptCore/jit/ThunkGenerators.cpp
  • Source/JavaScriptCore/llint/LLIntThunks.cpp
  • Source/JavaScriptCore/runtime/Options.cpp
  • Source/JavaScriptCore/runtime/OptionsList.h
  • Source/WTF/wtf/PtrTag.h
  • Source/WebKit/WebProcess/WebProcess.cpp
  • Tools/Scripts/run-jsc-stress-tests

Patch Preview

diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h
index 6925f04bc3d6..3f9233b2de58 100644
--- a/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h
+++ b/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h
@@ -65,6 +65,12 @@ class MacroAssemblerARM64E : public MacroAssemblerARM64 {
 
     ALWAYS_INLINE void tagPtr(PtrTag tag, RegisterID target)
     {
+        if (!tag) {
+            m_assembler.pacizb(target);
+            return;
+        }
+
+        RELEASE_ASSERT(Options::allowNonSPTagging());
         auto tagGPR = getCachedDataTempRegisterIDAndInvalidate();
         move(TrustedImm64(tag), tagGPR);
         m_assembler.pacib(target, tagGPR);
@@ -76,11 +82,17 @@ class MacroAssemblerARM64E : public MacroAssemblerARM64 {
             m_assembler.pacibsp();
             return;
         }
+        RELEASE_ASSERT(Options::allowNonSPTagging());
         m_assembler.pacib(target, tag);
     }
 
     ALWAYS_INLINE void untagPtr(PtrTag tag, RegisterID target)
     {
+        if (!tag) {
+            m_assembler.autizb(target);
+            return;
+        }
+
         auto tagGPR = getCachedDataTempRegisterIDAndInvalidate();
         move(TrustedImm64(tag), tagGPR);
         m_assembler.autib(target, tagGPR);
diff --git a/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp b/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
index 4e0d005cc08a..a597735bfaf7 100644
--- a/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
+++ b/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
@@ -254,6 +254,16 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
     ASSERT(JITCode::isBaselineCode(jit.baselineCodeBlock()->jitType()));
     jit.storePtr(AssemblyHelpers::TrustedImmPtr(jit.baselineCodeBlock()), AssemblyHelpers::addressFor(CallFrameSlot::codeBlock));
 
+    GPRReg returnPCReg = GPRInfo::regT5;
+#if CPU(ARM64E)
+    GPRReg signingTagReg = GPRInfo::regT2;
+    if (!Options::allowNonSPTagging()) {
+        returnPCReg = ARM64Registers::lr;
+        signingTagReg = MacroAssembler::stackPointerRegister;
+        // We could save/restore lr here but we don't need to because the LLInt/Baseline will load it from the stack before returning anyway.
+    }
+#endif
+
     const CodeOrigin* codeOrigin;
     for (codeOrigin = &exit.m_codeOrigin; codeOrigin && codeOrigin->inlineCallFrame(); codeOrigin = codeOrigin->inlineCallFrame()->getCallerSkippingTailCalls()) {
         InlineCallFrame* inlineCallFrame = codeOrigin->inlineCallFrame();
@@ -266,15 +276,25 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
 
         if (!trueCaller) {
             ASSERT(inlineCallFrame->isTail());
-            jit.loadPtr(AssemblyHelpers::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()), GPRInfo::regT3);
+            jit.loadPtr(AssemblyHelpers::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()), returnPCReg);
 #if CPU(ARM64E)
+            if (!Options::allowNonSPTagging()) {
+                JIT_COMMENT(jit, "lldb dynamic execution / posix signals could trash your stack"); // We don't have to worry about signals because they shouldn't fire in WebContent process in this window.
+                jit.move(MacroAssembler::stackPointerRegister, GPRInfo::regT4);
+            }
+
             jit.addPtr(AssemblyHelpers::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, GPRInfo::regT2);
-            jit.untagPtr(GPRInfo::regT2, GPRInfo::regT3);
-            jit.addPtr(AssemblyHelpers::TrustedImm32(inlineCallFrame->returnPCOffset() + sizeof(void*)), GPRInfo::callFrameRegister, GPRInfo::regT2);
-            jit.validateUntaggedPtr(GPRInfo::regT3, GPRInfo::regT4);
-            jit.tagPtr(GPRInfo::regT2, GPRInfo::regT3);
+            jit.untagPtr(GPRInfo::regT2, returnPCReg);
+            jit.validateUntaggedPtr(returnPCReg, GPRInfo::regT2);
+            jit.addPtr(AssemblyHelpers::TrustedImm32(inlineCallFrame->returnPCOffset() + sizeof(CPURegister)), GPRInfo::callFrameRegister, signingTagReg);
+            jit.tagPtr(signingTagReg, returnPCReg);
+
+            if (!Options::allowNonSPTagging()) {
+                JIT_COMMENT(jit, "lldb dynamic execution / posix signals are ok again");
+                jit.move(GPRInfo::regT4, MacroAssembler::stackPointerRegister);
+            }
 #endif
-            jit.storePtr(GPRInfo::regT3, AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
+            jit.storePtr(returnPCReg, AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
             jit.loadPtr(AssemblyHelpers::Address(GPRInfo::callFrameRegister, CallFrame::callerFrameOffset()), GPRInfo::regT3);
             callerFrameGPR = GPRInfo::regT3;
         } else {
@@ -291,10 +311,20 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
             }
 
 #if CPU(ARM64E)
-            jit.addPtr(AssemblyHelpers::TrustedImm32(inlineCallFrame->returnPCOffset() + sizeof(void*)), GPRInfo::callFrameRegister, GPRInfo::regT2);
-            jit.move(AssemblyHelpers::TrustedImmPtr(jumpTarget.untaggedPtr()), GPRInfo::regT4);
-            jit.tagPtr(GPRInfo::regT2, GPRInfo::regT4);
-            jit.storePtr(GPRInfo::regT4, AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
+            if (!Options::allowNonSPTagging()) {
+                JIT_COMMENT(jit, "lldb dynamic execution / posix signals could trash your stack"); // We don't have to worry about signals because they shouldn't fire in WebContent process in this window.
+                jit.move(MacroAssembler::stackPointerRegister, GPRInfo::regT4);
+            }