Skip to content

Explicit-dst clobber refresh wrongly applies to store (st) operations #36

Description

@ricardojrdez

Follow-up to #34 / #35.

The side-effect refresh added in RopChain._construct_ropchain.backtrack (PR #35) zeroes the clobber counter of a gadget's dst whenever a step writes it, so later steps can reuse that register:

norm_dst = arch.normalize_reg(gad.dst) if gad.dst else None
saved_dst = side_effected[norm_dst] if norm_dst else 0
if saved_dst:
    side_effected[norm_dst] = 0

This is correct for register writes (mov dst, src, lc(dst), …), but wrong for store operations.

Problem

st is mov [dst], src, so gad.dst holds the base register of the address ([rax]rax). That register is not written — it is consumed as an address. Zeroing its clobber count marks it as "freshly produced" when it is not, so a stale clobber from an earlier step can be silently masked, potentially yielding a ROP chain where the store's address register no longer holds the expected value.

Not a regression (a store's dst was never protected by the side-effect guard, which only checks effective_src), but the refresh should not apply here.

Suggested fix

Only apply the refresh when dst is a register actually written by the gadget, not a memory base. Options:

  • skip the refresh when the matched operation's dst operand is op_mem, or
  • guard on regs_write (only refresh if norm_dst is in the gadget's written registers).

Note: a regression test for the correct store behaviour should accompany the fix (the existing tests/test_ropchain.py::test_explicit_dst_clears_clobbered_register covers the register-write case).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions