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

Crash on certain input files #2105

Open
mrexodia opened this Issue Jan 20, 2019 · 9 comments

Comments

Projects
None yet
2 participants
@mrexodia
Copy link
Member

mrexodia commented Jan 20, 2019

ping @Mattiwatti

Some time ago I got a sample that will crash when loading it into x64dbg. The reason appears to be that RtlImageDirectoryEntryToData returns a garbage pointer. Probably the best solution is to rewrite those functions, or perform the same check as the loader does in order to reject the result.

@mrexodia

This comment has been minimized.

Copy link
Member Author

mrexodia commented Jan 20, 2019

Another report here, probably a similar issue but I didn't try yet... https://twitter.com/AGertani/status/1085900976039571457

@mrexodia mrexodia added the bug label Jan 20, 2019

@mrexodia

This comment has been minimized.

Copy link
Member Author

mrexodia commented Jan 20, 2019

I 'fixed' this issue in 7d53b1a, but just by wrapping the code in a try/except block... feels bad

@Mattiwatti

This comment has been minimized.

Copy link
Contributor

Mattiwatti commented Jan 21, 2019

try/except may very well be how the real loader handles it too... both the UM and KM loader are full of blocks like that. Though I don't have the magical ability to say which blocks require it and which don't. Possibly just one, possibly all of them...

I haven't got x64dbg on hand here, which directory was it that RtlImageDirectoryEntryToData was giving the bogus results for?

@mrexodia

This comment has been minimized.

Copy link
Member Author

mrexodia commented Jan 21, 2019

@Mattiwatti Mattiwatti referenced a pull request that will close this issue Jan 21, 2019

Open

DBG: more robust validation of PE directory sizes #2106

@Mattiwatti

This comment has been minimized.

Copy link
Contributor

Mattiwatti commented Jan 21, 2019

OK, I found the cause, it was a fairly straightforward dirSize > imageSize case and the directory pointer was being dereferenced without checking if that was the case. Essentially the same thing that was already fixed for the debug directory previously. Made a PR.

I do still get this though, but only when debugging from VS, and if I ignore the AVs everything works fine:
crash

No problems when debugging the exe straight or debugging x32dbg and then debugging the exe in the second instance.

I can't download the malware sample from VirusTotal btw, any chance you could check that? I tried downloading it in two different browsers but I just get forwarded to the homepage.

@mrexodia

This comment has been minimized.

Copy link
Member Author

mrexodia commented Jan 21, 2019

@Mattiwatti

This comment has been minimized.

Copy link
Contributor

Mattiwatti commented Jan 21, 2019

Oh yeah, w.r.t. try/except... according to my 'sources' if you catch my drift, ntdll does indeed wrap access to at least the export directory in LdrpGetProcedureAddress in a try, but only with a finally to do some cleanup. So in this case it would return an uninitialized NTSTATUS value! Keep in mind that my, erm, guesses re: this, are from very old Windows versions so this is likely fixed by now. But yeah, try/except doesn't seem such a bad idea.

@mrexodia

This comment has been minimized.

Copy link
Member Author

mrexodia commented Jan 21, 2019

@mrexodia

This comment has been minimized.

Copy link
Member Author

mrexodia commented Jan 21, 2019

I was thinking about the cleanup too, but probably it's good enough to just clear the exports/imports/whatever. Also another issue I see how is that I call vector.reserve with exportDir->NumberOfFunctions, so that could take up a good chunk of memory and cause a DoS too, ooops.

Mattiwatti added a commit to Mattiwatti/x64dbg that referenced this issue Jan 22, 2019

DBG: ReadExportDirectory: do bounds checks on all export dir entries …
…before indexing into arrays

Fixes x64dbg#2105 (second case/malware sample)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment