Skip to content

su locator misses valid lea register variants (rdx/rbx/rbp): the 3 RIP-relative locators accept inconsistent register sets #49

Description

@guclumhg

Summary

The three "find the RIP-relative lea that references string X" locators in the
codebase accept different sets of destination registers, and the one used by
the guest-su patcher is the narrowest. As a result, _find_isdevmode_entry()
does not locate an isDeveloperMode() whose entry lea targets rdx, rbx or
rbp — even though the instruction shape and the string target are otherwise
identical to the recognized rax/rcx/rsi/rdi cases.

Because _find_isdevmode_entry() is the unpatched locator that
su_patch_offline.enable() relies on (via _classify_elf_su), an su variant
that happens to use one of those three registers would be silently skipped:
no su is patched for that image, so apps on that Android version don't get
root, and no error is raised (the flow just reports "no gated su found / boot the
instance once first").

This is latent today — the bundled fallback signature (su_patch_offline.py,
_FALLBACK_SIGS = [[0x53, 0x48, 0x8D, 0x3D, ...]]) is push rbx; lea rdi, so
the current shipping su binaries appear to use rdi/rsi. But BlueStacks
rebuilds HD-*/guest binaries frequently, and register allocation for this
lea is exactly the kind of thing a compiler/codegen change flips. This is the
same "works today, silently reverts on a rebuild" failure class that #35
hardened the engine patch against, so it seems worth closing here too.

The inconsistency (three locators, three register rules)

All three do the same conceptual thing — scan for 48/4C 8D <modrm> <rel32> and
keep the one whose target == str_va — but disagree on which modrm bytes count
as a valid RIP-relative lea:

Location Accepted registers modrm test
su_patch.py:111 (_find_isdevmode_entry, unpatched su) rax, rcx, rsi, rdi (4) data[i+1] in (0x05, 0x0D, 0x3D, 0x35)
integrity_patch.py:194,200 (_locate_isdiskverify) all 8 data[i+1] in rip_modrm
su_patch_offline.py:336 (_classify_elf_su, already-patched su) any (all 8) (elf[i+3] & 0xC7) == 0x05

For RIP-relative addressing the modrm byte is (reg << 3) | 0x05 (mod=00,
rm=101), so the four bytes at su_patch.py:111 are exactly {rax, rcx, rsi, rdi} and the missing ones are rdx (0x15), rbx (0x1D), rbp (0x2D) (and
rsp, which is never a lea destination here).

Two consequences:

  1. enable() can't patch an su the patched-detector could recognize.
    _classify_elf_su recognizes an already-b0 01 c3-patched su whose
    orphaned lea uses any register (& 0xC7 == 0x05, line 336), but the
    unpatched path that would have had to create that patch
    (_find_isdevmode_entry, line 111) only accepts 4 registers. The two
    recognizers disagree.

  2. Silent skip on the GUI path. enable()_scan_su_entries
    _classify_elf_su_find_isdevmode_entry. A miss here just drops the su
    from the result set with no error.

Reproduction (self-contained, no BlueStacks needed)

These modules are pure-Python (no pywin32/PyQt import), so this runs anywhere.
It builds a minimal ELF64 with push rbx; lea <reg>,[rip+rel] pointing at the
real DEVMODE_STRING, varying only the destination register:

import struct, sys; sys.path.insert(0, ".")
import su_patch

DEVSTR = su_patch.DEVMODE_STRING
REGS = {"rax":0x05,"rcx":0x0D,"rdx":0x15,"rbx":0x1D,"rbp":0x2D,"rsi":0x35,"rdi":0x3D}

def build_elf64(modrm):
    VBASE, phoff, F, S = 0x400000, 0x40, 0x80, 0x100
    total = S + len(DEVSTR)
    b = bytearray(total)
    b[0:4] = b"\x7fELF"; b[4] = 2; b[5] = 1; b[6] = 1
    struct.pack_into("<H", b, 0x10, 2); struct.pack_into("<H", b, 0x12, 0x3E)
    struct.pack_into("<Q", b, 0x20, phoff)
    struct.pack_into("<H", b, 0x36, 56); struct.pack_into("<H", b, 0x38, 1)
    struct.pack_into("<I", b, phoff+0, 1)                 # PT_LOAD
    struct.pack_into("<Q", b, phoff+8, 0)                 # p_offset
    struct.pack_into("<Q", b, phoff+16, VBASE)            # p_vaddr
    struct.pack_into("<Q", b, phoff+32, total)            # p_filesz
    b[F], b[F+1], b[F+2], b[F+3] = 0x53, 0x48, 0x8D, modrm  # push rbx; lea reg,[rip+..]
    rel = (VBASE + S) - (VBASE + (F+1) + 7)
    struct.pack_into("<i", b, F+4, rel)
    b[S:S+len(DEVSTR)] = DEVSTR
    return bytes(b)

for name, modrm in REGS.items():
    got = su_patch._find_isdevmode_entry(build_elf64(modrm))
    print(f"{name:4} 0x{modrm:02X}  {'FOUND @0x%X' % got if got is not None else 'None  <-- MISSED'}")

Output:

rax  0x05  FOUND @0x80
rcx  0x0D  FOUND @0x80
rdx  0x15  None  <-- MISSED
rbx  0x1D  None  <-- MISSED
rbp  0x2D  None  <-- MISSED
rsi  0x35  FOUND @0x80
rdi  0x3D  FOUND @0x80

Suggested fix

Widen su_patch.py:111 to the full RIP-relative set, ideally sharing one
constant with integrity_patch.py:194 so the two can't drift again:

# both files
_RIP_MODRM = (0x05, 0x0D, 0x15, 0x1D, 0x25, 0x2D, 0x35, 0x3D)
...
if i >= 1 and data[i - 1] in (0x48, 0x4C) and data[i + 1] in _RIP_MODRM:

This is strictly safe: the candidate is still gated by the exact-target check
lea_vaddr + 7 + rel == str_vaddr, so widening the register set adds coverage
for more registers pointing at the same string without adding false positives —
integrity_patch.py:200 already relies on exactly that. The bundled
_FALLBACK_SIGS lea rdi entry could likewise be generalized, but the primary
locator fix is the important one.

Scope note: this is the 64-bit path. The 32-bit path (8D 83, lea eax,[ebx+disp]) is eax-only in both su_patch.py and su_patch_offline.py,
so it's internally consistent and left as-is.

Also worth noting: the tests/ suite arriving in #45 covers the UI/views and
engine rules but not the su/ELF locators, so this class of regression would
still ship undetected — a couple of synthetic-ELF cases like the repro above
would lock it down.

Happy to open a PR with the shared-constant change + a small regression test if
you'd like — flagging it as an issue first per the contributing note about
discussing changes.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions