Skip to content

xtensa: fix swapped isIntN arguments in decodeOffset_16_16Operand#2986

Open
jmestwa-coder wants to merge 2 commits into
capstone-engine:nextfrom
jmestwa-coder:xtensa-isintn-arg-swap
Open

xtensa: fix swapped isIntN arguments in decodeOffset_16_16Operand#2986
jmestwa-coder wants to merge 2 commits into
capstone-engine:nextfrom
jmestwa-coder:xtensa-isintn-arg-swap

Conversation

@jmestwa-coder

Copy link
Copy Markdown
Contributor

Your checklist for this pull request

  • I've documented or updated the documentation of every API function and struct this PR changes.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

decodeOffset_16_16Operand passes the untrusted immediate as the width argument to isIntN:

  • the call is isIntN(Imm, 8), but the signature is isIntN(unsigned N, int64_t x), so Imm is used as the bit count
  • the three sibling offset decoders correctly use isIntN(16, Imm)
  • Imm is a 4-bit field, so Imm == 0 shifts by (unsigned)(0 - 1) inside isIntN, which UBSan flags at MathExtras.h:62; it also wrongly rejects offsets 1..4

Swapped the arguments back to isIntN(8, Imm).

Test plan

ee.ldf.128.ip with a zero offset field reaches it: cstool -d esp32 2f704484. On an ASAN/UBSan build the old code aborts with shift exponent 4294967295 is too large; after the fix it decodes clean as ee.ldf.128.ip f8, f9, f7, f4, a2, 0. Added test_ub_isintn_xtensa_offset to tests/integration/test_poc.c for that word.

Closing issues

@github-actions github-actions Bot added the Xtensa Arch label Jul 4, 2026

@Rot127 Rot127 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

cc @b1llow

@Rot127 Rot127 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry, clicked to fast. Please add the instruction to issues.yml as test case

Signed-off-by: Syed Mohammed Nayyar <jmestwa@gmail.com>
@jmestwa-coder

Copy link
Copy Markdown
Contributor Author

added the word to issues.yaml as a test case (ee.ldf.128.ip f8, f9, f7, f4, a2, 0). passes under cstest.

Comment thread tests/issues/issues.yaml
address: 0x0
expected:
insns:
- asm_text: "ee.ldf.128.ip f8, f9, f7, f4, a2, 0"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please also test the details. More test coverage is always better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants