Vector35 / binaryninja-api

Public API, examples, documentation and issues for Binary Ninja
https://binary.ninja/
MIT License
900 stars 204 forks source link

armv7 lifting of multiply instructions is incorrect #4364

Closed D0ntPanic closed 11 months ago

D0ntPanic commented 1 year ago

The armv7 lifter produces IL expressions where the size of a multiply operation does not match either of the input operands. This causes the analysis of optimized division to always fail to simplify into a division expression in armv7 binaries.

Several mulitply instructions are lifted in this way. All of the multiplication operations need to verified and fixed so that the operand sizes match the operation size.

armv7

Example binary:

armv7.zip

lwerdna commented 1 year ago

All of the multiplication operations need to verified and fixed so that the operand sizes match the operation size.

Isn't the size of the multiplication operation the size of the result, and should therefore be the sum of the operands' sizes? relevant docs

lwerdna commented 11 months ago

Actual fix is at https://github.com/Vector35/arch-armv7/commit/8fb4292d846bdfc6818169f5abf9f7ca0a64f1ea.

image

We agreed in meeting:

Which was clarified in our documentation at https://github.com/Vector35/binaryninja-api/commit/2f415e3da3a417160c07928cc8b91f42eddbac3f

A few additional multiply instructions were fixed in https://github.com/Vector35/arch-armv7/commit/8fb4292d846bdfc6818169f5abf9f7ca0a64f1ea and https://github.com/Vector35/arch-armv7/commit/59f92c3c551f65f04d5c10920e2625930a5b3faf but they don't seem to be critical for these magic constant division situations.