You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Jan 22, 2024. It is now read-only.
Note that this issue is talking about a problem in the Metasploit-Javapayload repo, but for GitHub linking purposes, I'm dropping this here. It's probably not ideal, but neither is the division between the repos (or the names). - @todb-r7
Java Meterpreter calling sys.process.execute with arguments seems to fail. For example,
It looks like there are two versions of stdapi_sys_process_execute_V1_3.javastdapi_sys_process_execut.java in the repo. I don't understand why we have two versions or which is being currently built.
V1_3 doesn't append the args to the command. Both use Runtime.exec(string) instead of Runtime.exec(string[]) which would probably be better!
@jlee-r7 commented: As I understand it, the V1_3 version is for backwards compatibility. Mihi would know more.
@schierlm commented: In fact, v1_3 is for "forward" compatibility. If your Java version is 1.3 or higher, V1_3 is loaded, else the "versionless" version is loaded. Works the same with other versions. That way, all files without V1_* in name are 1.2 compatible (since the stage class loading requires at least 1.2 there is no point to not use it), all files with V1_3 are 1.3 compatible, and so on, which makes automatic API level verifying easier than if it was the other way round :) Which versions exist for a command are declared in the Loader class for the extension, at the moment there can be at most three, but we don't need more either.
About the concrete situation: The only difference between 1.3 and before should be that since 1.3 the current working directory can be set for spawned processes (so if you cd somewhere and then start a shell you get the shell there).
But even that does not work, since it got broken in this commit: rapid7/metasploit-javapayload@78ad111 , because of the way Java resolves overridden methods. The method in the main version was changed to take a String, but the override for Java 1.3 and up still takes a String array, therefore does not override anything at all (it is effectively dead code, and if we ran some dead code analysis, or had unit tests and coverage testing, we would see that). At the moment, the behavior of 1.3 and newer is therefore crippled and it will behave the same in all versions. And both versions will append the args to the command, but with no separator (space) in between.
I don't want decide which exec method should be used, but since I made the experience that the version using the string tends to cause more subtle bugs (e. g. on Unix systems, it will just split the string at spaces with no way to escape them, and pass the result to system()), I usually try to keep the argument list separate everywhere. Which does not really help since the Meterpreter API is built for Windows and takes two strings (command and arguments) instead. And when using it interactively, I don't need it to spawn more than a shell in most cases anyway :)
@jlee-r7 commented: This is a problem not only with the meterpreter API, but also the meterpreter command console. The execute command is designed in a away that matches the cmd, args API, so from user input, we never really have arguments separated to begin with. We could probably restructure execute to take all options at the beginning and then the rest of the command line as arguments, which might make it possible to have an args array.
Alternatively, we could have java figure out which shell to use (/bin/sh, /system/bin/sh, or cmd.exe) and do something like:
This issue was RM8742, originally filed by by @Meatballs1
Note that this issue is talking about a problem in the Metasploit-Javapayload repo, but for GitHub linking purposes, I'm dropping this here. It's probably not ideal, but neither is the division between the repos (or the names). - @todb-r7
Java Meterpreter calling sys.process.execute with arguments seems to fail. For example,
fails, but
would work.
It looks like there are two versions of
stdapi_sys_process_execute_V1_3.java
stdapi_sys_process_execut.java
in the repo. I don't understand why we have two versions or which is being currently built.V1_3 doesn't append the args to the command. Both use
Runtime.exec(string)
instead ofRuntime.exec(string[])
which would probably be better!@jlee-r7 commented: As I understand it, the V1_3 version is for backwards compatibility. Mihi would know more.
@schierlm commented: In fact, v1_3 is for "forward" compatibility. If your Java version is 1.3 or higher, V1_3 is loaded, else the "versionless" version is loaded. Works the same with other versions. That way, all files without V1_* in name are 1.2 compatible (since the stage class loading requires at least 1.2 there is no point to not use it), all files with V1_3 are 1.3 compatible, and so on, which makes automatic API level verifying easier than if it was the other way round :) Which versions exist for a command are declared in the Loader class for the extension, at the moment there can be at most three, but we don't need more either.
About the concrete situation: The only difference between 1.3 and before should be that since 1.3 the current working directory can be set for spawned processes (so if you cd somewhere and then start a shell you get the shell there).
But even that does not work, since it got broken in this commit: rapid7/metasploit-javapayload@78ad111 , because of the way Java resolves overridden methods. The method in the main version was changed to take a String, but the override for Java 1.3 and up still takes a String array, therefore does not override anything at all (it is effectively dead code, and if we ran some dead code analysis, or had unit tests and coverage testing, we would see that). At the moment, the behavior of 1.3 and newer is therefore crippled and it will behave the same in all versions. And both versions will append the args to the command, but with no separator (space) in between.
I don't want decide which exec method should be used, but since I made the experience that the version using the string tends to cause more subtle bugs (e. g. on Unix systems, it will just split the string at spaces with no way to escape them, and pass the result to system()), I usually try to keep the argument list separate everywhere. Which does not really help since the Meterpreter API is built for Windows and takes two strings (command and arguments) instead. And when using it interactively, I don't need it to spawn more than a shell in most cases anyway :)
@jlee-r7 commented: This is a problem not only with the meterpreter API, but also the meterpreter command console. The
execute
command is designed in a away that matches thecmd, args
API, so from user input, we never really have arguments separated to begin with. We could probably restructureexecute
to take all options at the beginning and then the rest of the command line as arguments, which might make it possible to have an args array.Alternatively, we could have java figure out which shell to use (/bin/sh, /system/bin/sh, or cmd.exe) and do something like:
This solution is more like how PHP and Python do it.
The text was updated successfully, but these errors were encountered: