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

[pe] List exports by ordinal as well as by name #125

Merged
merged 1 commit into from Apr 20, 2017

Conversation

lucasg
Copy link
Contributor

@lucasg lucasg commented Apr 20, 2017

PeViewer does not list exports by ordinal.

ClangDll.def

LIBRARY

EXPORTS  
   fun1		@1  NONAME 
   fun2		@2
   fun3		@3  NONAME   
   fun4		@4
   data1	@5
   data2	@6  NONAME

ClangDll.zip

Results :

  • Ida :
    ida_export_parsing

  • Depends :
    depends_export_parsing

  • Peviewer:
    peview_master_export_parsing

The "bug" was considering Exports->OrdinalTable as the ordinal table for all exports, whereas it's only for named exports.

Peviewer with "fixed" exports listing :
peview_pe_export_parsing

NB : there is probably a bug with how the VA are computed, since it's not entirerly consistent with ida or depends.

@dmex dmex merged commit bcda65a into winsiderss:master Apr 20, 2017
dmex added a commit that referenced this pull request Apr 20, 2017
@lucasg lucasg deleted the parse_exports_by_ordinals branch April 20, 2017 20:37
@dmex
Copy link
Member

dmex commented Apr 20, 2017

Looks good, thanks!

@dmex
Copy link
Member

dmex commented Apr 27, 2017

there is probably a bug with how the VA are computed

user32.dll is a good example of unnamed exports with invalid VA:
http://i.imgur.com/3IyrUN0.png

@lucasg
Copy link
Contributor Author

lucasg commented Apr 27, 2017

I've looked into it yesterday and here's my guess : unlike most PE parsers (dumpbin.exe, pefile, etc.) which works on the file itself, phlib map the PE in memory. I don't know why it has an impact on the computed VA, but it does.

If you look at radare2, it compute the exports address the following way :

It makes no sense, since the official PE-COFF spec sheet explicitely say it's a rva.

Anyway here's the patch :

diff --git a/phlib/mapimg.c b/phlib/mapimg.c
index 05d179f..eae0ce4 100644
--- a/phlib/mapimg.c
+++ b/phlib/mapimg.c
@@ -741,11 +741,7 @@ NTSTATUS PhGetMappedImageExportFunction(
     }
     else
     {
-        Function->Function = PhMappedImageRvaToVa(
-            Exports->MappedImage,
-            rva,
-            NULL
-            );
+       Function->Function = (PVOID)((ULONG_PTR) rva);
        Function->ForwardedName = NULL;
     }

@dmex
Copy link
Member

dmex commented Apr 28, 2017

the official PE-COFF spec sheet explicitly say it's a rva

@wj32 would know more about it... I think PhGetMappedImageExportFunction was originally designed with a different purpose hence why it returned different data.

I've committed your patch for now. It does break plugin compatibility but I don't think anything other than peview was using the function so there shouldn't be any issues changing it 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants