-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Negated structure offsets #573
Comments
Any progress on this? |
From the blog entry, I understand the issue in general. Do you have some example bytes from a function that exhibits this behavior. I have an example I've coded up to see if it matches what you are seeing. I just want to make sure the changes in consideration will solve this issue and others related to this in other github issues. |
Original: //
// CONTAINING_RECORD macro
// Value of structure member (field), given the (type) and the List_Entry (head) in original containing type
//
#define CONTAINING_RECORD(address, type, field) (\
(type *)((char*)(address) -(unsigned long)(&((type *)0)->field)))
typedef struct _LIST_ENTRY {
struct _LIST_ENTRY *Flink;
struct _LIST_ENTRY *Blink;
} LIST_ENTRY, *PLIST_ENTRY;
typedef struct _LDR_DATA_TABLE_ENTRY
{
struct _LIST_ENTRY InLoadOrderLinks;
struct _LIST_ENTRY InMemoryOrderLinks;
struct _LIST_ENTRY InInitializationOrderLinks;
void* DllBase;
void* EntryPoint;
} LDR_DATA_TABLE_ENTRY, *PLDR_DATA_TABLE_ENTRY;
int countIt(PLDR_DATA_TABLE_ENTRY list) {
int count = 0;
PLIST_ENTRY PListHead = &(list->InMemoryOrderLinks);
PLIST_ENTRY pEntry = PListHead->Flink;
while(pEntry != PListHead)
{
PLDR_DATA_TABLE_ENTRY entry = CONTAINING_RECORD(pEntry,LDR_DATA_TABLE_ENTRY,InMemoryOrderLinks);;
printf("num1=%x %x\n", entry->DllBase, entry->EntryPoint);
count++;
pEntry = pEntry->Flink;
}
return count;
} Decompiled Output: ulong countIt(PLDR_DATA_TABLE_ENTRY param_1)
{
_LIST_ENTRY *p_Var1;
uint count;
p_Var1 = (param_1->InMemoryOrderLinks).Flink;
count = 0;
while (¶m_1->InMemoryOrderLinks != p_Var1) {
count = count + 1;
printf("num1=%x %x\n",p_Var1[2].Flink,p_Var1[2].Blink);
p_Var1 = p_Var1->Flink;
}
return (ulong)count;
} |
This affects both the decompiler and the listing view.
Your example is reproducing the problem perfectly.
What other related Github issues? I searched but couldn't find any relating to negative structure offsets. Maybe they run under a different term. But I would like to take a look. |
Can I just say that CONTAINING_RECORD isn't the only thing that would cause this? Sometimes the compiler generates code where a structure is accessed from another "base" address. So for example:
Would look like:
But obviously this gets crazy with non trivial types.. |
I'm wondering if there's any progress on this? Also @0x6d696368 did you find a workaround (except creation of a new struct by cutting out first members, which is awful inconvenient)? |
Unfortunately, no. |
Even though I agree with all the above, IDA 7.2 actually improved it even more with shifted pointers. See https://www.hex-rays.com/products/ida/support/idadoc/1695.shtml and https://www.hex-rays.com/products/ida/7.2/index.shtml. Basically, you can define int *__shifted(mystruct,20) myptr; where: struct mystruct
{
char buf[16];
int dummy;
int value; // <- myptr points here
double fval;
}; I think having that in Ghidra too would solve all the issues above, and generically. |
Has there been any progress on this issue? Can we help in any way? |
@emteere, are you working on this or is the community needing to think how to fix it? |
@saruman9 I think it's the latter. |
An equivalent to IDA's shifted pointers in Ghidra is also something I'm finding myself increasingly in need of. Hopefully someone manages to figure something out. |
For everyone needing this (including me): I'm currently working on implementing shifted pointers, expect a PR in the next few days ;) |
Update: I now have a working implementation (I'm going to create a PR this afternoon), but it isn't ready to merge yet (mainly because 3% of the integration tests fail, and there are no new unit tests) |
Second Update: The PR (#2189) is now out! Might take a while to merge though, because it affects so much of the codebase. |
What is the ghidra base commit you used in case we want to test your PR? |
I rebased my commit right before pushing it, so it should be the current HEAD of the master branch PS: There was actually a merge conflict, because in the week I was working on the patch, there was actually a change to one part of the code I was simultaneously working on |
Still a problem as of 10.1. Really need this feature :/ |
Would be really nice to have this feature!!! |
I'll join you here: Such a feature is really needed as soon as possible. From my personal experience gathered so far, the analysis is very limited because you have to "convert" everything first to get to the originally meant field. From my point of view it is essential for the analysis of OOP with inheritance. |
I noticed that a similar problem occurs when dealing with some functions that were compiled as C++ virtual member functions. For example in the case of |
I think this issue affects also thread-local variables. See the example code and how it is decompiled by ghidra: tls.c compile with #include <threads.h>
#include <stdio.h>
#include <assert.h>
thread_local int foo = 0;
int main(void) {
assert(scanf("%d", &foo) == 1);
printf("foo is: %d\n", foo);
return 0;
} It would be nice to be able to define the thread-local variables + stack canary when they are used. |
Is your feature request related to a problem?
When code traverses a multiple linked list data structure, e.g. traversing
_PEB_LDR_DATA
viaInInitializationOrderLinks
:You get pointers to the above structure with an offset of
0x10
(offset ofInInitializationOrderLinks
).If you give this pointer the type
struct _LDR_DATA_TABLE_ENTRY
all the offsets are obviously wrong.Describe the solution you'd like
IDA handles this by allowing the user to supply a struct offset, see: https://www.hexblog.com/?p=63
So you hit
T
on the usage of the struct and define the offset0x10
and you get your correct types.Describe alternatives you've considered
Currently I copy the old structure and create a new structure with its name prefixed by the offset
_0x010
and deleting the first 2 entries in the structure.However, this is:
Additional context
When working with linked list data structures this features is needed often.
The text was updated successfully, but these errors were encountered: