Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support setting a data variable to volatile to prevent the analysis from using its current value #6267

Open
bb33bb opened this issue Dec 20, 2024 · 6 comments
Labels
Component: Core Issue needs changes to the core Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround Type: Enhancement Issue is a small enhancement to existing functionality

Comments

@bb33bb
Copy link

bb33bb commented Dec 20, 2024

Version and Platform (required):

  • Binary Ninja Version: 4.3.6594-dev Personal (c7f7c5e4)
  • OS: manjaro
  • OS Version: Linux version 6.11.10-2-MANJARO (linux611@manjaro) (gcc (GCC) 14.2.1 20240910, GNU ld (GNU Binutils) 2.43.0) use Caches folder on OS X for download cache to prevent backups #1 SMP PREEMPT_DYNAMIC Mon, 25 Nov 2024 05:29:44 +0000
  • CPU Architecture: [e.g. x64 or M1]

Bug Description:
there is really no offensive for ninja, using ida is only for compareing code to check the difference
i buyed ninja, and i love ninja.
Here is the following code reversed by ida

__int64 __fastcall sub_2EF0(unsigned int a1, void *a2, __int64 a3, __int64 a4)
{
  __int64 v5; // [xsp+8h] [xbp-28h] BYREF
  _QWORD v6[2]; // [xsp+10h] [xbp-20h] BYREF
  void *v7; // [xsp+20h] [xbp-10h]
  unsigned int v8; // [xsp+2Ch] [xbp-4h]

  v8 = a1;
  v7 = a2;
  v6[1] = a3;
  v6[0] = a4;
  binder_read(a1, (__int64)a2, a3, v6);
  v5 = 0LL;
  binder_read_buffer_lookup(v7, v6[0], 0x80407202, (void **)&v5);
  return *(_QWORD *)(v5 + 48);
}

we can compare the last lien of code reversed by ida and ninja.
Here is ninja reversed

00002ef0    int64_t sub_2ef0(int32_t arg1, int32_t* arg2, int64_t arg3, int64_t arg4)

00002ef0    {
00002ef0        int64_t var_30 = arg4;
00002f20        sub_3f28(arg1, arg2, arg3, &var_30);
00002f40        int32_t var_3c = sub_47bc(arg2, var_30, 0x80407202);
00002f54        return 0x38004000000000;
00002ef0    }

ninja give the value to retrun.
it is 0x38004000000000
obvious it is not so confidential to say ninja is right.
the code is arm64 arch and we can check it as following

00002ef0    int64_t sub_2ef0(int32_t arg1, int32_t* arg2, int64_t arg3, int64_t arg4)

00002ef0  ff0301d1   sub     sp, sp, #0x40
00002ef4  fd7b03a9   stp     x29, x30, [sp, #0x30] {__saved_x29} {__saved_x30}
00002ef8  fdc30091   add     x29, sp, #0x30 {__saved_x29}
00002efc  e80303aa   mov     x8, x3
00002f00  a0c31fb8   stur    w0, [x29, #-0x4 {var_14}]
00002f04  a1031ff8   stur    x1, [x29, #-0x10 {var_20}]
00002f08  e20f00f9   str     x2, [sp, #0x18 {var_28}]
00002f0c  e3430091   add     x3, sp, #0x10 {var_30}
00002f10  e80b00f9   str     x8, [sp, #0x10 {var_30}]
00002f14  a0c35fb8   ldur    w0, [x29, #-0x4 {var_14}]
00002f18  a1035ff8   ldur    x1, [x29, #-0x10 {var_20}]
00002f1c  e20f40f9   ldr     x2, [sp, #0x18 {var_28}]
00002f20  02040094   bl      sub_3f28
00002f24  e3230091   add     x3, sp, #0x8 {var_38}
00002f28  ff0700f9   str     xzr, [sp, #0x8 {var_38}]  {0x0}
00002f2c  a0035ff8   ldur    x0, [x29, #-0x10 {var_20}]
00002f30  e10b40f9   ldr     x1, [sp, #0x10 {var_30}]
00002f34  42408e52   mov     w2, #0x7202
00002f38  0208b072   movk    w2, #0x8040, lsl #0x10  {0x80407202}
00002f3c  20060094   bl      sub_47bc
00002f40  e00700b9   str     w0, [sp, #0x4 {var_3c}]
00002f44  e80740f9   ldr     x8, [sp, #0x8 {var_38}]
00002f48  001940f9   ldr     x0, [x8, #0x30]  {0x38004000000000}
00002f4c  fd7b43a9   ldp     x29, x30, [sp, #0x30] {__saved_x29} {__saved_x30}
00002f50  ff030191   add     sp, sp, #0x40
00002f54  c0035fd6   ret     
@Zerotistic
Copy link
Contributor

obvious it is not so confidential to say ninja is right.

I am not sure to understand this issue, what is binja doing wrong/badly and what is the expected result?

@bb33bb
Copy link
Author

bb33bb commented Dec 20, 2024

00002ef0    int64_t sub_2ef0(int32_t arg1, int32_t* arg2, int64_t arg3, int64_t arg4)

00002ef0    {
00002ef0        int64_t var_30 = arg4;
00002f20        sub_3f28(arg1, arg2, arg3, &var_30);
00002f40        int32_t var_3c = sub_47bc(arg2, var_30, 0x80407202);
00002f54        return 0x38004000000000; //  this line is not OK
00002ef0    }

i think this line is not right.

obvious it is not so confidential to say ninja is right.

I am not sure to understand this issue, what is binja doing wrong/badly and what is the expected result?

@Zerotistic
Copy link
Contributor

Ah yeah, ok, sorry for my misunderstanding I got confused. Can you share the binary (you might need to zip it) ?

@psifertex
Copy link
Member

Yeah, the binary would help here -- I recreated the disassembly shown in isolation and it doesn't show the same presumably because of a lack of global data variables:

Screenshot 2024-12-20 at 12 00 03

@Zerotistic
Copy link
Contributor

If this helps, I think it's a Binder transaction (judging from the renaming in the IDA decomp) and that value 0x38004000000000 is being loaded from offset 0x30 in a buffer after a BR_TRANSACTION command (0x80407202). Looking at the Android source, it's probably reading from a binder_transaction_data structure, where offset 0x30 contains a union (either a pointer to more data or an inline buffer). Since this is following a pointer to transaction data, its does seem strange that binja would assume it returns a constant value

The 0x38004 part in that big number might be some kind of type or enum that gets shifted into the final value, but yeah, without the binary it's hard to say why binja thinks this specific value would always be returned

@bb33bb
Copy link
Author

bb33bb commented Dec 21, 2024

If this helps, I think it's a Binder transaction (judging from the renaming in the IDA decomp) and that value 0x38004000000000 is being loaded from offset 0x30 in a buffer after a BR_TRANSACTION command (0x80407202). Looking at the Android source, it's probably reading from a binder_transaction_data structure, where offset 0x30 contains a union (either a pointer to more data or an inline buffer). Since this is following a pointer to transaction data, its does seem strange that binja would assume it returns a constant value

The 0x38004 part in that big number might be some kind of type or enum that gets shifted into the final value, but yeah, without the binary it's hard to say why binja thinks this specific value would always be returned

yes, give me a short time.
it is a little sensative.
after i make a demo , i will upload it.
i am really appreciate!

@xusheng6 xusheng6 changed the title too eary to calculate the value in stack Support setting a data variable to volatile to prevent the analysis from using its current value Dec 23, 2024
@xusheng6 xusheng6 added Type: Enhancement Issue is a small enhancement to existing functionality Component: Core Issue needs changes to the core Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround labels Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Issue needs changes to the core Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround Type: Enhancement Issue is a small enhancement to existing functionality
Projects
None yet
Development

No branches or pull requests

4 participants