Crashes were occuring when the underlying channel had no more output
because the value of the `bytesRead` variable was not set to zero.
Consumers of the function assumed that bytesRead was value if non-zero.
POSIX would also hang when unsupported commands are executed, this
commit changes this so that a response is returned when the command
isn't supported.
This exploit has been causing crashes and BSODs on various systems and
hence is deemed too unstable to be included in the default deployment of
Meterpreter. `getsystem` should only contain code which attempts to get
SYSTEM privileges via safe means; it should not have exploits in it.
This commit removes kitrap0d from `getsystem`. The code will be moved to a
windows local exploit in MSF instead.
Note: webcam stuff doesn't work on Windows 2012. Work needs to be done to
make it work on this platform because the DX classes used to do the webcam
capture are not present on the system.
Does as it says on the tin. Various tweaks made to source and to project
files to make the builds come out with ZERO warnings.
Let's keep it clean from here!
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.
This changeset brings windows into line with the last set of POSIX
changes. With this changeset we are now in a position where both POSIX and
Windows are able to create and open interactive channels, put them in the
background, and terminate them without crashing, hanging or leaving
processes running behind the scenes.
POSIX was out of whack with Windows as a result of the changes made
around channels. The schedular in posix was very different, and this
commit brings it into line.
Other than the obvious issues, a non-obvious issue with the changes
was that the channel was being freed up on close prior to the thread
terminating. This doesn't appear to be an issue on Windows, but was
causing crashes on close in POSIX.
The changes go quite deep. This changeset requires a lot of testing.
The goals of this work are:
* To fix issue where backgrounding and re-interacting with channels wasn't
working.
* To fix issue where closing of meterpreter was not closing off background
prcoesses (such as cmd.exe).
The two things preventing this stuff from working were:
* When interactive channels are backgrounded their handles were destroyed
along with the context that wraps them up. Making them interactive again
had no impact because the handle and context were invalid. If anything,
this made meterpreter unstable. Sometimes the session would die when
attempting to interact with the channel again.
* When closing channels, there was no way of terminating the process that
sat behind the scenes because no reference to the process was retained.
Channels would close and handles would close, but no process termination
was done.
To fix these problems:
* The interactive thread no longer terminates when backgrounded. Instead
its put in a suspended state where it's waiting a signal from a resume
handle that's associated with the channel's context. This means that the
destruction of the context doesn't happen at all until the termination
of the channel, which is exactly when it should happen anyway.
* Process handles are stored alongside the input/output handles so that
when the time comes, the process can be terminated if required. This
means that when the channels are closed, the code has a reference to the
associated process which can be terminated. This is only done for
interactive processes, non-interactive processes do not have this
problem because meterpreter doesn't have to keep track of them.
Previous change stopped the session from crashing on NULL, but this change
actually has more realistic messages coming back intead of always
indicating that things happened correctly. It's still up to the caller to
check the return code to make sure it's ok
* 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`.
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!
Fix issue where the railgun API was relyling on FormatMessage returning a
valid pointer when the error is `ERROR_SUCCESS`. On some platforms, such
as XP SP3, the function would return a NULL pointer for this case. This
fix makes sure that in the case of a NULL pointer the error message is set
to a value that matches that found on other platforms.
[FixRM 8505]
