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

Non-sensical operand primary reference in Listing #4669

Open
ubitux opened this issue Oct 16, 2022 · 11 comments
Open

Non-sensical operand primary reference in Listing #4669

ubitux opened this issue Oct 16, 2022 · 11 comments
Assignees
Labels
Status: Triage Information is being gathered

Comments

@ubitux
Copy link

ubitux commented Oct 16, 2022

Describe the bug
Sometimes the operand primary reference makes no sense in the Listing view.

To Reproduce
Steps to reproduce the behavior:

  1. gunzip cm001.gz and load+analyze cm001 into Ghidra
  2. Jump to 0x401703
  3. Observe RSI referring to some random string

This function is the init function of the glibc (__libc_csu_init) and has nothing to do with the password string displayed here. In doubt, this can be verified with the source code of the crackme available at https://github.com/JCWasmx86/Crackme/tree/main/001 (it's simply using a constructor attribute, causing the creation of a 2nd init function).

Workaround: go to options, Listing FieldsOperands Fields, uncheck "Always Show Primary Reference"

Expected behavior
The operand primary reference should not be inferred here.

Screenshots
2022-10-16-232421-Gaipe3ii

Attachments
cm001.gz

Environment (please complete the following information):

  • OS: Archlinux
  • Java Version: 11.0.16.1
  • Ghidra Version: 10.1.5
  • Ghidra Origin: Archlinux official packages
@emteere
Copy link
Contributor

emteere commented Oct 17, 2022

The reference most likely came from a flow into this function from the validate.s routine which loads RSI with the string reference.

Does the last syscall of the validate routine immediately precede this function, such that it would flow into this function?
The syscall is currently implemented to not to fall through to the next instruction.

If this is the case you can modify the flow of the syscall to be non-flowing into this function, and re-run analysis. With the flow removed, the reference should not be created. There is some current work on syscalls, however I'm not sure they would take care of this issue.

The references are not normally random, and can come from flow that either exists or existed earlier when the particular section of code was analyzed. It does occasionally make mistakes from overzealous propagation of constants, or incorrect internal flow assumptions.

@ubitux
Copy link
Author

ubitux commented Oct 17, 2022

You are correct, the previous function flows into that one because of the syscalls (last one being an exit). I used the recommended x86 syscalls script, which seems to adjust the flow overrides:

-- CALLOTHER(syscall) Call Override: exit (syscall::0000003c)
-- Flow Override: CALL_RETURN (FALL_THROUGH)

Unfortunately, re-running the analysis (a) didn't update the operand reference. If I clear the instruction (c) then decompile it again (d) the reference goes away, but that works without the flow override as well, and re-running the analysis restores the wrong reference.

I tried to override the flow manually but the interface was kind of confusing to me, I wasn't exactly sure how to proceed.

@emteere
Copy link
Contributor

emteere commented Oct 17, 2022

Overriding psuedo-ops is not easy. It is mainly useful on instructions that already have some sort of flow.

If you clear it, the instruction is analyzed in the function it currently resides.
It might actually come back if you clear an instruction in the validate() function above init, or if you reanalyze the whole program without changing the flow of the instruction.

You can override the FallThru on the syscall as well.

@ryanmkurtz ryanmkurtz added the Status: Triage Information is being gathered label Oct 18, 2022
@ubitux
Copy link
Author

ubitux commented Oct 19, 2022

You can override the FallThru on the syscall as well.

Yeah (and I believe that's what the x86 syscall script does), but unless I misunderstood something that's not enough to prevent flow fall-through into the next function.

@emteere
Copy link
Contributor

emteere commented Oct 27, 2022

I don't think the X86 syscall overrides the fallthru, as a syscall can return after the syscall.

If you simply clear the instruction at the with the bad reference, it shouldn't come back.
We don't always clear analysis references, although we will be clearing some with re-analysis in the near future.

Hope this clears up the issue for you. Not ideal, but not an unexpected type of issue to occur in SRE.

@emteere emteere added Status: Waiting on customer Waiting for customer feedback and removed Status: Triage Information is being gathered labels Oct 27, 2022
@ubitux
Copy link
Author

ubitux commented Oct 28, 2022

Ah, clearing the padding instructions following the syscall does prevent the fallthrough, and the reference doesn't come back. But I guess I'm lucky there is padding in the first place.

I still don't get why the "Flow Override: CALL_RETURN (FALL_THROUGH)" generated by the script doesn't seem to be enough. Am I misunderstanding its meaning?

fallthru.mp4

@ryanmkurtz ryanmkurtz added Status: Triage Information is being gathered and removed Status: Waiting on customer Waiting for customer feedback labels Oct 31, 2022
@emteere
Copy link
Contributor

emteere commented Nov 9, 2022

The script should be enough, but you would still need to clear the bad reference. When constant analysis happens again on the function the reference shouldn't come back, if I'm understanding correctly.

@emteere emteere added Status: Waiting on customer Waiting for customer feedback and removed Status: Triage Information is being gathered labels Nov 9, 2022
@ubitux
Copy link
Author

ubitux commented Nov 9, 2022

The script should be enough, but you would still need to clear the bad reference. When constant analysis happens again on the function the reference shouldn't come back, if I'm understanding correctly.

It does come back if the padding instructions between the 2 functions is decompiled (the NOP after the syscall, at 004016b8 which you can see at t=12s in the video)

@ubitux
Copy link
Author

ubitux commented Nov 10, 2022

Here is a demo with + without the padding to show that it impacts analysis even though the script has adjusted the fallthrough:

ghidra-ref.mp4

@ryanmkurtz ryanmkurtz added Status: Triage Information is being gathered and removed Status: Waiting on customer Waiting for customer feedback labels Nov 12, 2022
@emteere
Copy link
Contributor

emteere commented Jan 3, 2023

OK, I finally see the issue you are bringing up.

Modifying the flow of an instruction can technically only occur if the instruction actually has flow. A call to a pseudo-op like syscall() doesn't count. Only a GOTO or CALL pcode op would count as flow.

I'll have to think about that. I think the way it is done now the SYSCALL is assumed to return, even with the script to calculate the call system function.

@emteere
Copy link
Contributor

emteere commented Jan 3, 2023

You can set the Fallthrough to NULL using Fallthru->Set... in the popup menu on the instruction.
You can do this after running the syscall resolving script.

The decompiler doesn't consider the fall-thru overrides, however normal flow analysis does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Information is being gathered
Projects
None yet
Development

No branches or pull requests

3 participants