Skip to content

fix: restrict UID spoofing exclusively to the TEESimulator daemon to prevent root process crashes#42

Open
aartzz wants to merge 7 commits into
Enginex0:mainfrom
aartzz:main
Open

fix: restrict UID spoofing exclusively to the TEESimulator daemon to prevent root process crashes#42
aartzz wants to merge 7 commits into
Enginex0:mainfrom
aartzz:main

Conversation

@aartzz

@aartzz aartzz commented Jul 12, 2026

Copy link
Copy Markdown

Problem

Currently, the binder_interceptor indiscriminately spoofs all binder transactions originating from sender_euid == 0 (root) to UID 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 as cameraserver, vold, init, etc.) when they attempt to communicate with keystore2.

On many devices (especially MediaTek devices), this indiscriminate spoofing causes severe side effects:

  • Camera HAL Breakage: cameraserver requests to keystore fail authorization because they are spoofed as UID 1000, leading to a SIGABRT crash (queryMinBufNum failure in MTK camera pipelines).
  • Soft Bootloops: Essential processes like vold crash during boot when accessing keystore2 due to intercepted and hijacked requests.

Solution

This PR introduces a more precise UID spoofing mechanism:

  1. PID Memorization: The interceptor now memorizes the precise PID of the TEESimulator daemon (g_daemon_pid) during the initial backdoor registration phase (intercept::kBackdoorCode).
  2. Selective Spoofing: UID spoofing (sender_euid = 1000) is now strictly limited to transactions that originate from this specific g_daemon_pid.
  3. Safe Fallthrough: All other root processes retain their original UID 0 and are explicitly excluded from hijacking (hijack = false), allowing them to interact securely and directly with keystore2 without tampering.

Testing

  • Verified that the TEESimulator daemon can still successfully interface with keystore2.
  • Confirmed that the MTK camera (cameraserver) works flawlessly alongside the module (no SIGABRT).
  • Verified that the device boots successfully without any vold-related soft bootloops.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of privileged daemon communication by limiting UID spoofing/interception to the daemon identified during the handshake.
    • Added dynamic bypass configuration so camera-related apps can bypass interception automatically.
    • Updated device attestation checks to rely on overall TEE functionality instead of per-algorithm hardware probing.
    • Prevented boot-time clearing of log size properties and disabled Soter supervisor startup during app initialization.
  • Chores
    • Updated built-in updater release metadata/download link for the next package version.
    • Delayed daemon startup by 30 seconds before launching the app.

…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.
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Runtime 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.

Changes

Runtime behavior changes

Layer / File(s) Summary
Startup context and bypass setup
app/src/main/java/org/matrix/TEESimulator/App.kt, module/daemon, app/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.kt
Stores the prepared application context, delays daemon startup by 30 seconds, disables Soter supervisor startup, and discovers camera-related UIDs for bypass configuration.
Binder bypass protocol
app/src/main/java/org/matrix/TEESimulator/interception/core/BinderInterceptor.kt, app/src/main/cpp/binder_interceptor.cpp
Adds a Binder backdoor transaction for sending bypass UIDs and stores up to 32 received UIDs in the native interceptor.
Daemon-aware Binder transaction filtering
app/src/main/cpp/binder_interceptor.cpp
Stores the daemon PID during the backdoor handshake, bypasses configured traffic, and rewrites sender_euid to 1000 only for matching daemon transactions.
Attestation changes
app/src/main/java/org/matrix/TEESimulator/attestation/DeviceAttestationService.kt
Makes RSA and EC attestability checks return isTeeFunctional without hardware probing.
Boot-time log property handling
module/service.sh
Disables clearing persistent log buffer size properties after boot completion.

Release metadata update

Layer / File(s) Summary
Release download metadata
module/update.json
Changes the version to v6.0.1-310, the version code to 305, and the archive URL to a v6.0.1-305 path with a v6.0.1-310 filename.

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
Loading

Suggested reviewers: enginex0

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: limiting UID spoofing to the daemon to avoid root-process crashes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6d241e5 and 11809cf.

📒 Files selected for processing (1)
  • app/src/main/cpp/binder_interceptor.cpp

Comment thread app/src/main/cpp/binder_interceptor.cpp Outdated
Comment on lines +375 to +385
// 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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/ -S

Repository: 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d5d551e3-127f-487c-878a-ea177b67079c

📥 Commits

Reviewing files that changed from the base of the PR and between 11809cf and 2f08509.

📒 Files selected for processing (1)
  • module/update.json

Comment thread module/update.json Outdated
Comment on lines +3 to +4
"versionCode": 304,
"zipUrl": "https://github.com/aartzz/TEESimulator-RS/releases/download/v6.0.1-304/TEESimulator-RS-v6.0.1-309-Release.zip",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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 . || true

Repository: 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.

xzc0215 pushed a commit to xzc0215/TEESimulator-RS that referenced this pull request Jul 16, 2026
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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/src/main/java/org/matrix/TEESimulator/App.kt (1)

66-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove 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-out SoterProcessSupervisor.start(systemContext) call and its preceding obsolete comment.
  • module/service.sh#L14-L22: Remove the commented-out subshell block that clears the persist.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

📥 Commits

Reviewing files that changed from the base of the PR and between 2f08509 and 9a54307.

📒 Files selected for processing (5)
  • app/src/main/cpp/binder_interceptor.cpp
  • app/src/main/java/org/matrix/TEESimulator/App.kt
  • app/src/main/java/org/matrix/TEESimulator/attestation/DeviceAttestationService.kt
  • module/service.sh
  • module/update.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/src/main/cpp/binder_interceptor.cpp

Comment on lines +105 to +112
// 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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 ProbeSpec data class and its instances (rsaTeeProbe, rsaStrongBoxProbe, ecTeeProbe, ecStrongBoxProbe)
  • The state maps attestableVerdicts and attestProbesInFlight
  • The private functions isHardwareAttestable, probeAttestability, isAttestationUnavailable, and deleteProbeKey

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.

github-actions Bot and others added 2 commits July 17, 2026 17:17
…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 'Збій камери'.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
app/src/main/cpp/binder_interceptor.cpp (1)

601-615: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Track successfully read UIDs instead of relying on the requested count.

If data.readInt32(&uid) fails for any entry, the corresponding index in g_dynamic_bypass_uids will remain unchanged (potentially retaining a stale UID), but g_dynamic_bypass_count will still be set to the full requested count. This could result in unintended bypasses if stale UIDs are left within the boundary.

Consider incrementing an actual_count only 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 | 🔵 Trivial

Delaying 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 sleep with a polling loop that waits for a specific system property (e.g., via getprop) 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 value

Remove 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2e17273 and 62e8299.

📒 Files selected for processing (5)
  • app/src/main/cpp/binder_interceptor.cpp
  • app/src/main/java/org/matrix/TEESimulator/App.kt
  • app/src/main/java/org/matrix/TEESimulator/interception/core/BinderInterceptor.kt
  • app/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.kt
  • module/daemon

Comment on lines +379 to +381
// 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Comment on lines +130 to +132
if (bypassUids.isNotEmpty()) {
setBypassUids(backdoor, bypassUids)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
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
aartzz and others added 2 commits July 20, 2026 21:32
- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant