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

Fix race condition and remote hwbp removal (#44) #45

Closed
wants to merge 1 commit into from

Conversation

dauthleikr
Copy link

This fixes everything from #44 except the first detection vector. Turns out I don't need that one after all.
To keep other processes from being able to remove debug registers, I used the same logic already implemented in HookNtGetContextThread: Allow the removal unless the caller is "hidden" too. Which means that if whatever you are debugging has multiple processes, just hide all of them (could use the GUI) and you're fine.

Please test carefully :)

…g registers, fix race condition for stripping ContextFlags
ULONG OriginalContextFlags = 0;
if(IsHidden)
CONTEXT contextCopy;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, but I now remember why I didn't implement this. It's undocumented, but the sizeof(CONTEXT) isn't actually the maximum size a user might pass to Get/SetThreadContext. See https://github.com/x64dbg/TitanEngine/blob/x64dbg/TitanEngine/TitanEngine.Debugger.Context.cpp#L987, but I'm unfortunately not sure how to deal with this...

Copy link
Author

@dauthleikr dauthleikr Mar 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I have two possible solutions in mind. I took a look at what InitializeContext does, and it calls a function called RtlGetExtendedContextLength2 to get the length for its ContextFlags.

Solution 1: Figure out the maximum size and go with that
Solution 2: The same functions also exists in the kernel, add it to the other undocumented functions and use it.

I can do one of those when I have some spare time, right now the bad fix works for me.

@dauthleikr
Copy link
Author

Closing this for now, a proper implementation is not trivial, and I doubt anybody really abuses this specific vector

@dauthleikr dauthleikr closed this Apr 10, 2020
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.

2 participants