This repository has been archived by the owner on Jan 22, 2024. It is now read-only.
Post VS2013 merges for #34, #39, #42, #43, and #44 #46
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Webcam code was sometimes causing crashes in Meterpreter when attempting to stop the camera after a frame had been captured. This appeared to be because the thread that started the capture was not the same thread that ended it. CoInitialize() and CoUninitialize() need to be called on the same thread and objects created on one thread need to be released on the same thread. This change results in a new worker thread that is used for the lifetime of the capture, and the callers have to interact with this thread via basic threading events. This is currently "proof of concept" code, rather than production-ready code. The goal is to see if it solves the issue on the known targets before tidying it up and locking it down for release.
This work contains a bunch of changes around command dispatching. The goals for this bit of work were to: * Provide the ability for commands to be executed on the same thread as the server rather than always creating new threads and executing them on those threads. * Have the means for _special_ commands, such as `exit` and `migrate` to shut down the server cleanly without having to rely on signalling across threads or by doing brutal thread termination via shared global handles. This should not only fix the dirty shutdown problem on Windows which leaves tasks dangling (or based on the prior attempt at fixing, crashing stuff as well), it should also help clean up the shutdown process in POSIX. These changes hit a very important part of Meterpreter and so should be reviewed with intense scrutnity. I expect this PR to garner a log of critique and most likely a number of changes before being included in the main line. The `PacketDispatcher` was modified to include a new function pointer called an `inline_handler`. This new member indicates that there's a handler which should be invoked inline. While this sits alongside the existing `handler`, they are actually mutually exclusive. If an `inline_handler` is specified then the `handler` is ignored and it is assumed that the command is intended to be handled inline. The signature of the inline handler is different to the standard handler, and this is why a new function pointer was added rather than a simple flag. Addition of this parameter meant that the basic command structure changed, and that obviously affects all of the extensions and their respective commands. This changeset therefore updates each of those command declarations so that they use the new macros that hide this detail. Other things to be mindful of: * This version of the code reads the command's `method` prior to invoking any other function, and after that the command itself is passed around to the threaded or non-threaded routes for invocation. An extra thread parameter was included as as result, and an overload for the `thread_create` function was added which supported this new parameter. This was named `thread_create3` because `thread_create_with_another_paramter` sounded a bit crap. * The migration code, which originally had a `thread_kill` and an event wait once the new meterpreter session had been created, has been modified to not do any waiting at all. Instead it finishes execution as fast as possible and returns control to the server which should respond by shutting down in a clean way. * Originally the code always attempted to call a command handler in the base command list and then, if found, would also call an "overload" in the extension commands list. From the investigation that I did, it appears that the overloaded methods did nothing in the base (they'd early out during invocation). As a result, the new way of doing things acts like a 'true' overload in that the extension commands are searched first, and if one is found this is the command that is executed. Any base commands with the same method name will not get executed. In the case where there is no extension command found, the base command list is then queried. If a command is found that command is instead invoked. * The POSIX version still compiles cleanly, but I've never been able to build a version that runs on my machines. I'm not sure if there's a trick to getting POSIX builds to run, and if there is I don't know it. Whoever scrutinises this build should make sure that the POSIX version that they build can still run and (hopefully) exit cleanly. I've added lots of documentation, but there's always room for improvement. Hopefully this will fix the `*_tcp` side of Redmine 8438. Bring on the feedback!
* Updated `thread_create` so that it has 3 parameters, and removed `thread_create3`. * Updated all calls to `thread_create` and added the extra parameter of `NULL`. * Fixed comment typo. * Removed assignment where value is not used. * Checked for `NULL` prior to setting the result. * Undefined `DEBUGTRACE`.
I'm such a noob. My grep-fu was weak with this one.
Make now correctly loads the environment automatically if it can find it.
Checking version info now has support for the later versions of windows based on the documentation available from MSDN.
This commit also contains fixes for proper extraction of subnet masks based on operating system.
Pulled the latest version of the incognito code from: http://labs.mwrinfosecurity.com/blog/2012/07/18/incognito-v2-0-released/ This included a fix for Windows 2003 x64, which was reported as a bug in RM 8281.
Conflicts: make.bat
This allows for system proxy setting to be pulled out. Windows-only at this point.
This commit tidies up the webcam code, adds documentation and adds a couple of small clean-ups and optimisations around resource usage. `audio.h` is not included in `webcam.h` any more as it's not needed at all by that code, however it has been added to `precomp.h` so that `stdapi.c` can use and see it along with the other files.
|
Test FAILED. |
|
Looks like the CI failed, thanks Metasploit Bot. Here's the failure: |
|
That would be #44. |
|
The command dispatcher changes have to be landed first, and then merged into the channel refactor. They both modify the thread interface stuff a bit unfortunately. Working with @jlee-r7 on the command dispatcher stuff. Hope to get that sorted really soon. Thanks Tod. |
|
So, the procedure for this PR-of-PRs is:
If that doesn't happen in a reasonable amount of time (24 hours from now?), then:
I don't want these other fixes lingering just because #34 and #38 are fighting. They're useful, demo'ed, and done. |
|
+1 from me. We're on it at the mo, hope to have the POSIX stuff nailed shortly. |
todb-r7
pushed a commit
to todb-r7/meterpreter
that referenced
this pull request
Nov 6, 2013
todb-r7
pushed a commit
to todb-r7/meterpreter
that referenced
this pull request
Nov 6, 2013
todb-r7
pushed a commit
to todb-r7/meterpreter
that referenced
this pull request
Nov 6, 2013
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR lands most of the outstanding PR's from @OJ, post-VS2013. Mainly, I'm proposing this PR in order to speed up validation testing.
This PR contains:
These all merged cleanly.
This PR does not contain PR #38, since that conflicted with #34 (but not master). That'll need to be sorted.
All of the above has been successfully demo'ed, so this PR should be landed so binaries can be rebuilt for distribution (assuming the Jenkins build passes).
Please do test the new functionality again, though, now that they're all together, just to be sure nothing got stomped in the merge fest.