fix: restrict UID spoofing exclusively to the TEESimulator daemon to prevent root process crashes#42
fix: restrict UID spoofing exclusively to the TEESimulator daemon to prevent root process crashes#42aartzz wants to merge 7 commits into
Conversation
…emon Previously, all binder transactions with a sender_euid of 0 (root) were indiscriminately spoofed to UID 1000. While this successfully bypassed keystore permissions for the TEESimulator daemon, it unintentionally intercepted and spoofed legitimate transactions from other root processes such as cameraserver, vold, and init. This indiscriminate spoofing caused severe side effects on some devices, such as breaking the camera HAL on MTK devices (crashing cameraserver) and causing soft bootloops due to vold crashes when accessing keystore2. This commit resolves the issue by memorizing the daemon's precise PID during the backdoor registration phase. UID spoofing is now strictly limited to transactions originating from the daemon's PID. All other root processes fall through without being hijacked or spoofed, retaining their original UID 0 and passing directly to keystore2.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRuntime startup, attestation checks, Binder bypass configuration and transaction filtering, boot-time log property handling, and release metadata were updated. Camera-related application UIDs can now be configured as Binder bypass UIDs, with credential rewriting restricted to transactions from the captured daemon PID. ChangesRuntime behavior changes
Release metadata update
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Keystore2Interceptor
participant BinderInterceptor
participant NativeBinderInterceptor
participant BinderTransaction
Keystore2Interceptor->>BinderInterceptor: send camera-related UIDs
BinderInterceptor->>NativeBinderInterceptor: transact SET_BYPASS_UIDS_CODE
NativeBinderInterceptor->>NativeBinderInterceptor: store bypass UIDs
BinderTransaction->>NativeBinderInterceptor: provide sender PID and EUID
NativeBinderInterceptor->>BinderTransaction: bypass hijack or spoof EUID to 1000
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/cpp/binder_interceptor.cpp`:
- Around line 375-385: The daemon PID state in the binder interception flow is
never invalidated after daemon termination. Update the teardown path associated
with the backdoor handshake to reset g_daemon_pid when the daemon exits, or
validate that the recorded PID is still the live daemon before the sender_euid
spoofing branch uses it; ensure recycled PIDs cannot trigger UID 1000 spoofing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 601d2547-29a7-47d8-96f7-8c3a61ac2d89
📒 Files selected for processing (1)
app/src/main/cpp/binder_interceptor.cpp
| // Save the daemon's PID | ||
| g_daemon_pid = txn_data->sender_pid; | ||
| // Check 2: Bypass interception for root requests, but spoof UID for the daemon | ||
| } else if (txn_data->sender_euid == 0) { | ||
| // The kernel driver fills sender_euid. | ||
| // libbinder.so trusts this value to populate IPCThreadState. | ||
| txn_data->sender_euid = 1000; | ||
| LOGV("[Hook] Spoofing UID for transaction: 0 -> %d", txn_data->sender_euid); | ||
| hijack = false; // Never hijack to avoid recursion | ||
| if (txn_data->sender_pid == g_daemon_pid) { | ||
| // The kernel driver fills sender_euid. | ||
| // libbinder.so trusts this value to populate IPCThreadState. | ||
| txn_data->sender_euid = 1000; | ||
| LOGV("[Hook] Spoofing UID for transaction: 0 -> %d", txn_data->sender_euid); | ||
| } | ||
| hijack = false; // Never hijack root processes (vold, init, cameraserver) to avoid recursion and bootloops |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for any code that resets or clears g_daemon_pid
rg -n 'g_daemon_pid' app/src/main/cpp/Repository: Enginex0/TEESimulator-RS
Length of output: 435
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== binder_interceptor.cpp around the g_daemon_pid logic =="
sed -n '200,410p' app/src/main/cpp/binder_interceptor.cpp
echo
echo "== Search for daemon lifecycle / cleanup / unregister paths =="
rg -n 'g_daemon_pid|unregister|unregister.*daemon|death|linkToDeath|unlinkToDeath|cleanup|reset.*pid|pid.*reset|daemon.*die|TEESimulator|mount\(' app/src/main/cpp/ app/src/main/ -SRepository: Enginex0/TEESimulator-RS
Length of output: 48360
Reset g_daemon_pid on daemon death. g_daemon_pid is only set during the backdoor handshake and never cleared, so a recycled PID could cause unrelated root transactions to get spoofed to UID 1000. Clear it on teardown or add a liveness check before spoofing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/cpp/binder_interceptor.cpp` around lines 375 - 385, The daemon
PID state in the binder interception flow is never invalidated after daemon
termination. Update the teardown path associated with the backdoor handshake to
reset g_daemon_pid when the daemon exits, or validate that the recorded PID is
still the live daemon before the sender_euid spoofing branch uses it; ensure
recycled PIDs cannot trigger UID 1000 spoofing.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@module/update.json`:
- Around line 3-4: Regenerate module/update.json using the packaging workflow
driven by app/build.gradle.kts, rather than editing its fields individually.
Ensure the generated version, versionCode, and zipUrl consistently use the same
canonical release metadata and artifact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| "versionCode": 304, | ||
| "zipUrl": "https://github.com/aartzz/TEESimulator-RS/releases/download/v6.0.1-304/TEESimulator-RS-v6.0.1-309-Release.zip", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'module/update.json' 'app/build.gradle.kts' || true
printf '\n== module/update.json ==\n'
if [ -f module/update.json ]; then
cat -n module/update.json
fi
printf '\n== app/build.gradle.kts outline ==\n'
if [ -f app/build.gradle.kts ]; then
ast-grep outline app/build.gradle.kts --view expanded || true
fi
printf '\n== search for version/tag/url generation ==\n'
rg -n --no-heading -S 'versionCode|zipUrl|version|release|download|aartzz|Enginex0|TEESimulator-RS' app/build.gradle.kts module/update.json . || trueRepository: Enginex0/TEESimulator-RS
Length of output: 47406
Regenerate module/update.json from the packaging workflow
version, versionCode, and zipUrl are out of sync here: the JSON mixes v6.0.1-307, 304, aartzz, and v6.0.1-309. That can make update checks treat the release as a downgrade or point users at the wrong artifact. app/build.gradle.kts already writes this file from the canonical release metadata, so update this file through that pipeline instead of editing the fields separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@module/update.json` around lines 3 - 4, Regenerate module/update.json using
the packaging workflow driven by app/build.gradle.kts, rather than editing its
fields individually. Ensure the generated version, versionCode, and zipUrl
consistently use the same canonical release metadata and artifact.
This resolves an issue introduced in 6193da0 where a `deleteKey` transaction for a software-generated key was incorrectly passed through to the hardware keystore. Since the hardware is unaware of such keys, this results in inconsistent state management. The success reply is formatted correctly without a result code, per the AIDL interface specification. Reference: https://cs.android.com/android/platform/superproject/main/+/main:out/soong/.intermediates/system/hardware/interfaces/keystore2/aidl/android.system.keystore2-V6-java-source/gen/android/system/keystore2/IKeystoreSecurityLevel.java;l=406
1. Prevented intercepting cameraserver and system apps by skipping UIDs < 10000 in binder_interceptor.cpp. 2. Disabled SoterProcessSupervisor in App.kt (WeChat/Chinese chats). 3. Bypassed hardware probes in DeviceAttestationService.kt to prevent TrustZone memory leaks during boot. 4. Cleaned up service.sh.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/org/matrix/TEESimulator/App.kt (1)
66-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove dead commented-out code.
Disabled functionality should be removed rather than left as commented-out code, which clutters the codebase and reduces maintainability. Version control retains the history if these snippets are ever needed again.
app/src/main/java/org/matrix/TEESimulator/App.kt#L66-L68: Remove the commented-outSoterProcessSupervisor.start(systemContext)call and its preceding obsolete comment.module/service.sh#L14-L22: Remove the commented-out subshell block that clears thepersist.logd.size*properties.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/org/matrix/TEESimulator/App.kt` around lines 66 - 68, Remove the obsolete commented-out SoterProcessSupervisor.start(systemContext) call and its preceding comment in app/src/main/java/org/matrix/TEESimulator/App.kt (lines 66-68). Also remove the commented-out subshell that clears persist.logd.size* properties in module/service.sh (lines 14-22); no replacement code is needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/org/matrix/TEESimulator/attestation/DeviceAttestationService.kt`:
- Around line 105-112: Remove the unused attestation probing infrastructure from
DeviceAttestationService, including ProbeSpec, all probe instances,
attestableVerdicts, attestProbesInFlight, and the private helpers
isHardwareAttestable, probeAttestability, isAttestationUnavailable, and
deleteProbeKey. Update isRsaAttestable and isEcAttestable and their upstream
KeyMintSecurityLevelInterceptor callers to remove the now-unused strongBox
parameter while preserving their isTeeFunctional-based behavior.
---
Nitpick comments:
In `@app/src/main/java/org/matrix/TEESimulator/App.kt`:
- Around line 66-68: Remove the obsolete commented-out
SoterProcessSupervisor.start(systemContext) call and its preceding comment in
app/src/main/java/org/matrix/TEESimulator/App.kt (lines 66-68). Also remove the
commented-out subshell that clears persist.logd.size* properties in
module/service.sh (lines 14-22); no replacement code is needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96829a30-e18c-4f32-8d78-772ef8ba71be
📒 Files selected for processing (5)
app/src/main/cpp/binder_interceptor.cppapp/src/main/java/org/matrix/TEESimulator/App.ktapp/src/main/java/org/matrix/TEESimulator/attestation/DeviceAttestationService.ktmodule/service.shmodule/update.json
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/cpp/binder_interceptor.cpp
| // MTK KeyMint on certain Samsung/Xiaomi devices permanently locks TrustZone secure memory | ||
| // if a dummy attestation key is generated by a system UID, causing the Camera HAL to crash | ||
| // later. Since EC and RSA attestation are mandatory if TEE is functional, we safely | ||
| // bypass the hardware probes and assume attestability based solely on TEE functionality. | ||
| fun isRsaAttestable(strongBox: Boolean): Boolean = isTeeFunctional | ||
|
|
||
| /** Whether the real hardware can attest an EC key at the requested security level. */ | ||
| fun isEcAttestable(strongBox: Boolean): Boolean = | ||
| isHardwareAttestable(if (strongBox) ecStrongBoxProbe else ecTeeProbe) | ||
| fun isEcAttestable(strongBox: Boolean): Boolean = isTeeFunctional |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove unused hardware probing logic.
Because isRsaAttestable and isEcAttestable now unconditionally return isTeeFunctional, the entire hardware probing infrastructure has become dead code.
Please remove the following to reduce complexity:
- The
ProbeSpecdata class and its instances (rsaTeeProbe,rsaStrongBoxProbe,ecTeeProbe,ecStrongBoxProbe) - The state maps
attestableVerdictsandattestProbesInFlight - The private functions
isHardwareAttestable,probeAttestability,isAttestationUnavailable, anddeleteProbeKey
Additionally, the strongBox parameter on these methods is now unused. Consider removing it if the upstream caller (KeyMintSecurityLevelInterceptor) can be updated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/org/matrix/TEESimulator/attestation/DeviceAttestationService.kt`
around lines 105 - 112, Remove the unused attestation probing infrastructure
from DeviceAttestationService, including ProbeSpec, all probe instances,
attestableVerdicts, attestProbesInFlight, and the private helpers
isHardwareAttestable, probeAttestability, isAttestationUnavailable, and
deleteProbeKey. Update isRsaAttestable and isEcAttestable and their upstream
KeyMintSecurityLevelInterceptor callers to remove the now-unused strongBox
parameter while preserving their isTeeFunctional-based behavior.
…njection - Add 30s delay in module/daemon to allow MediaTek cameraserver to fully initialize without being interrupted by ptrace during boot. - Implement dynamic bypass UIDs for camera packages to prevent binder interception overhead. - Whitelist system services (UID < 10000) from being intercepted by default, except NFC (1027). - Fixes issue where queryMinBufNum fails and camera app displays 'Збій камери'.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/src/main/cpp/binder_interceptor.cpp (1)
601-615: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTrack successfully read UIDs instead of relying on the requested count.
If
data.readInt32(&uid)fails for any entry, the corresponding index ing_dynamic_bypass_uidswill remain unchanged (potentially retaining a stale UID), butg_dynamic_bypass_countwill still be set to the full requestedcount. This could result in unintended bypasses if stale UIDs are left within the boundary.Consider incrementing an
actual_countonly on successful reads to ensure the bypass loop bounds strictly reflect populated data.♻️ Proposed fix
status_t BinderInterceptor::handleSetBypassUids(const Parcel &data) { int32_t count = 0; if (data.readInt32(&count) != OK) return BAD_VALUE; if (count < 0) return BAD_VALUE; if (count > 32) count = 32; + int32_t actual_count = 0; for (int32_t i = 0; i < count; i++) { int32_t uid = 0; if (data.readInt32(&uid) == OK) { - g_dynamic_bypass_uids[i].store(uid, std::memory_order_relaxed); + g_dynamic_bypass_uids[actual_count++].store(uid, std::memory_order_relaxed); } } - g_dynamic_bypass_count.store(count, std::memory_order_release); + g_dynamic_bypass_count.store(actual_count, std::memory_order_release); LOGI("Set %d dynamic bypass UIDs", count); return OK; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/cpp/binder_interceptor.cpp` around lines 601 - 615, Update BinderInterceptor::handleSetBypassUids to track an actual_count that increments only when data.readInt32(&uid) succeeds. Store each successfully read UID contiguously, publish actual_count through g_dynamic_bypass_count, and use it in the log instead of the requested count.module/daemon (1)
15-15: 🩺 Stability & Availability | 🔵 TrivialDelaying interception exposes a 30-second unpatched window at boot.
Adding a static 30-second delay means that any application or system component requesting attestation during this window will bypass the simulator and hit the real hardware keystore. This could expose the true device state or cause early-boot attestations to fail.
If this delay is specifically to allow MediaTek camera initialization to complete, consider replacing the static
sleepwith a polling loop that waits for a specific system property (e.g., viagetprop) to signal that initialization has finished. This would close the vulnerability window faster on devices that boot quickly and prevent races on slower devices.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@module/daemon` at line 15, Replace the fixed 30-second sleep in the daemon startup flow with polling for the relevant MediaTek camera initialization completion property via getprop. Continue only after the property signals readiness, while preserving a bounded retry or timeout so interception is not indefinitely delayed.app/src/main/java/org/matrix/TEESimulator/App.kt (1)
71-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out code.
Commented-out code should be removed to keep the codebase clean. If the Soter supervisor is intentionally disabled as per the PR objectives, consider removing the code entirely. If it's meant to be toggled in the future, consider putting it behind a feature flag or configuration option.
♻️ Proposed fix
- // Mount the SOTER forge on the on-demand soterserver process. The supervisor - // binds and (re)injects on its own thread, returning at once so it never blocks the loop. - // SoterProcessSupervisor.start(systemContext)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/org/matrix/TEESimulator/App.kt` at line 71, Remove the commented-out SoterProcessSupervisor.start call near the application startup code; if supervisor startup must remain optionally available, replace the comment with the project’s established feature flag or configuration mechanism rather than leaving dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/cpp/binder_interceptor.cpp`:
- Around line 379-381: Update the daemon lifecycle handling around g_daemon_pid
so it is cleared when the daemon terminates, or validate that the recorded PID
is still alive and remains the daemon before spoofing. Ensure stale or recycled
PIDs cannot trigger UID 1000 spoofing in the interception path.
In
`@app/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.kt`:
- Around line 130-132: Update the bypass UID handling around setBypassUids to
enforce the native handleSetBypassUids limit of 32 entries: detect when
bypassUids exceeds 32, log a warning, and pass only the first 32 UIDs while
preserving the existing behavior for smaller lists.
---
Nitpick comments:
In `@app/src/main/cpp/binder_interceptor.cpp`:
- Around line 601-615: Update BinderInterceptor::handleSetBypassUids to track an
actual_count that increments only when data.readInt32(&uid) succeeds. Store each
successfully read UID contiguously, publish actual_count through
g_dynamic_bypass_count, and use it in the log instead of the requested count.
In `@app/src/main/java/org/matrix/TEESimulator/App.kt`:
- Line 71: Remove the commented-out SoterProcessSupervisor.start call near the
application startup code; if supervisor startup must remain optionally
available, replace the comment with the project’s established feature flag or
configuration mechanism rather than leaving dead code.
In `@module/daemon`:
- Line 15: Replace the fixed 30-second sleep in the daemon startup flow with
polling for the relevant MediaTek camera initialization completion property via
getprop. Continue only after the property signals readiness, while preserving a
bounded retry or timeout so interception is not indefinitely delayed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94e741b2-015d-41f9-a0f5-13ec072c2d21
📒 Files selected for processing (5)
app/src/main/cpp/binder_interceptor.cppapp/src/main/java/org/matrix/TEESimulator/App.ktapp/src/main/java/org/matrix/TEESimulator/interception/core/BinderInterceptor.ktapp/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.ktmodule/daemon
| // Save the daemon's PID | ||
| g_daemon_pid = txn_data->sender_pid; | ||
| // Check 2: Bypass interception for specific system requests, but spoof UID for the daemon |
There was a problem hiding this comment.
Reset g_daemon_pid on daemon death.
g_daemon_pid is only set during the backdoor handshake and is never cleared. If the daemon process terminates, its PID could be recycled and assigned to another root or system process. Transactions from that new process would then be incorrectly spoofed to UID 1000, potentially leading to unauthorized access or system instability. Ensure g_daemon_pid is cleared during teardown or validate the PID's liveness before spoofing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/cpp/binder_interceptor.cpp` around lines 379 - 381, Update the
daemon lifecycle handling around g_daemon_pid so it is cleared when the daemon
terminates, or validate that the recorded PID is still alive and remains the
daemon before spoofing. Ensure stale or recycled PIDs cannot trigger UID 1000
spoofing in the interception path.
| if (bypassUids.isNotEmpty()) { | ||
| setBypassUids(backdoor, bypassUids) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent silently dropping bypass UIDs if there are more than 32 camera packages.
The native hook handleSetBypassUids accepts a maximum of 32 UIDs. If the device has more than 32 packages matching "camera", the excess UIDs will be silently dropped by the C++ interceptor, causing those remaining cameras to remain hooked and potentially fail initialization.
Consider explicitly clamping the list to 32 elements and logging a warning if the limit is exceeded so this edge case is visible in logs.
🐛 Proposed fix
if (bypassUids.isNotEmpty()) {
- setBypassUids(backdoor, bypassUids)
+ if (bypassUids.size > 32) {
+ org.matrix.TEESimulator.logging.SystemLogger.warning("Found more than 32 camera UIDs (${bypassUids.size}). Only the first 32 will be bypassed.")
+ }
+ setBypassUids(backdoor, bypassUids.take(32))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (bypassUids.isNotEmpty()) { | |
| setBypassUids(backdoor, bypassUids) | |
| } | |
| if (bypassUids.isNotEmpty()) { | |
| if (bypassUids.size > 32) { | |
| org.matrix.TEESimulator.logging.SystemLogger.warning("Found more than 32 camera UIDs (${bypassUids.size}). Only the first 32 will be bypassed.") | |
| } | |
| setBypassUids(backdoor, bypassUids.take(32)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.kt`
around lines 130 - 132, Update the bypass UID handling around setBypassUids to
enforce the native handleSetBypassUids limit of 32 entries: detect when
bypassUids exceeds 32, log a warning, and pass only the first 32 UIDs while
preserving the existing behavior for smaller lists.
…ofing - Dynamically detect if resetprop-rs is available in /data/adb/tricky_store/ - Pass the --stealth flag to ensure property modifications bypass deep detection (no serial bump, no futex wake) - Fallback to standard resetprop if resetprop-rs is absent - Update module version in update.json
- Downloaded pre-compiled resetprop-rs binaries for arm64-v8a, armeabi-v7a, x86, and x86_64 - Modified customize.sh to extract the appropriate binary to /data/adb/tricky_store/resetprop-rs during installation - Ensures stealth property spoofing is available out-of-the-box for all users
Problem
Currently, the
binder_interceptorindiscriminately spoofs all binder transactions originating fromsender_euid == 0(root) toUID 1000. While this approach successfully allows the TEESimulator daemon to bypass keystore permissions, it unintentionally intercepts and alters legitimate transactions from other vital system root processes (such ascameraserver,vold,init, etc.) when they attempt to communicate withkeystore2.On many devices (especially MediaTek devices), this indiscriminate spoofing causes severe side effects:
cameraserverrequests to keystore fail authorization because they are spoofed asUID 1000, leading to aSIGABRTcrash (queryMinBufNumfailure in MTK camera pipelines).voldcrash during boot when accessing keystore2 due to intercepted and hijacked requests.Solution
This PR introduces a more precise UID spoofing mechanism:
g_daemon_pid) during the initial backdoor registration phase (intercept::kBackdoorCode).sender_euid = 1000) is now strictly limited to transactions that originate from this specificg_daemon_pid.UID 0and are explicitly excluded from hijacking (hijack = false), allowing them to interact securely and directly withkeystore2without tampering.Testing
cameraserver) works flawlessly alongside the module (noSIGABRT).vold-related soft bootloops.Summary by CodeRabbit