binaryninja-api icon indicating copy to clipboard operation
binaryninja-api copied to clipboard

"Display as enum member" affects array index in the same line as the enum constant

Open crand6 opened this issue 1 year ago • 5 comments

Version and Platform (required):

  • Binary Ninja Version: 4.2.5811-dev
  • OS: Mac OS X
  • OS Version: 14.5
  • CPU Architecture: x64

Bug Description: In HLIL, when setting an integer constant to be displayed as an enum member, if an array index in the same line has the same value as the enum member, it will also be displayed as an enum member instead of left as an integer index.

Steps To Reproduce: Please provide all steps required to reproduce the behavior:

  1. Binary has an HLIL line such as pollfd_array[1].events |= 1
  2. Right click on the furthest right 1 and display as an enum member
  3. Both 1s in the HLIL line are converted to be displayed enum members

Expected Behavior: The array index should be left alone and only the 1 that I asked to display as an enum member should be updated.

Screenshots/Video Recording: Before: image

After: image

Note the issue does happen when the array index is a different value than the enum member in the same HLIL line (see the second line in the screenshots). My enumeration has valid values for 0 and 1, but I do not see the same issue when the array index is different than the enum value in the same line.

crand6 avatar Aug 13 '24 14:08 crand6

I have same issue. It isn't specific to array indicies. Seems like it updates all integer constants (having same value) on the same line.

ArtemPisarenko avatar Aug 16 '24 10:08 ArtemPisarenko

This is actually a complex one, which is related to how we store the display type and how we associate the integer at different IL levels

xusheng6 avatar Aug 19 '24 07:08 xusheng6

IMHO, bug impact is underestimated. It is very annoying in my use case, when I have a function with 4 integer arguments and a lot of callers which directly pass integer constants with vast majority of them to be 0, 1 or 2. Since such calls combined with argument list are placed on single line in HLIL and Pseudo C views, it significantly limits analysis. Although I was lucky enough to workaround issue by redefining 3 of 4 function arguments as different enum types, which correctly updated their representation at all caller places and left me with relaxed necessity to alter only one integer constant at each line. But it's just because of specific and simple function behaviour I was able to reverse-engineer and happily discover its arguments meaning allowing such redefinition. I'm sure there are still many use cases requiring to individually change displaying of many integer constants placed on one line (with same value but completely different meanings).

ArtemPisarenko avatar Aug 19 '24 16:08 ArtemPisarenko

Part of the reason for the "Low" impact rating is exactly what you discovered -- you can use enums to work-around the majority of instances where this would impact you.

psifertex avatar Aug 19 '24 16:08 psifertex

Related to https://github.com/Vector35/binaryninja-api/issues/3490

xusheng6 avatar Aug 20 '24 14:08 xusheng6