qtc-de / beanshooter

JMX enumeration and attacking tool.
GNU General Public License v3.0
378 stars 45 forks source link

Add support for tonka exec output decoding with specified charset #39

Open peter5he1by opened 8 months ago

peter5he1by commented 8 months ago

In the beginning beanshooter does not decode the output of tonka exec. It just writes the original bytes to console without any side effect. So I think it could be even better if we add an option to decode the output with specified charset to string!

PS: I kept the code format carefully because there is no editorconfig file.

qtc-de commented 8 months ago

Hi @peter5he1by :wave:

good stuff and kudos for investigating beanshooters option layout to place everything into the correct location :+1: I know, it is not that straight forward :sweat_smile:

I would like to make three more requests:

  1. Could you please retarget the PR to the develop branch? Easiest way is probably by cherry picking your commits to it. I will create a pull request template soon to mention that the dev branch should be targeted when users create a new PR. Sorry that it was not already in place.
  2. I think we should catch the UnsupportedEncodingException that could occur when creating the InputStreamReader. This gets currently implicitly caught by catching the IOException, but the error message will not be that clear. I think catching it separately and printing a nice error would be cool. Maybe something like this:
    Logger.printlnMixedYellow("The specified encoding", charset, "is not supported.");
    Logger.printlnMixedBlue("Supported charsets include:", <list of most common charsets>, "as well as supported charsets of your Java distribution");
  3. It is probably also a good idea to add some kind of validation before the command execution. It would probably a bad user experience if the command gets executed on the server, but the output is thrown away because an invalid charset was selected. Ideally such an error would be caught by some kind of validation beforehand. I think the most elegant way would be to add this validation on the argparse level in the OptionHandler. Something like the following should work:
    if( option == TonkaBeanOption.EXEC_CHARSET )
    {
        List<String> supportedCharsets = new ArrayList<String>();
        supportedCharsets.addAll(Charset.availableCharsets().keySet());
        arg.choices(supportedCharsets);
    }

    Although this may explode in the help screen, but we will see ;)

peter5he1by commented 8 months ago

Validation process in TonkaBean Dispatcher

Well I got a new idea and tried. It looks good.

If the charset is unsuppotted, the command will be executed anyway but the output could be saved.

https://github.com/peter5he1by/beanshooter/commit/29a73a49d6e4d02c6b2b2cb5d76ffd5cdc210dfc

图片

The validation process above is embedded in the TonkaBean Dispatcher class (after the command executed).

Validation process in OptionHandler via Argument.choices

yeah it will explode in the help message if we set all available charsets as choices:

if (option == TonkaBeanOption.EXEC_CHARSET)
{
    arg.choices(new ArgumentChoice()
    {
        @Override
        public boolean contains(Object o)
        {
            try
            {
                return null != o && Charset.isSupported((String) o);
            }
            catch (IllegalCharsetNameException e)
            {
                return false;
            }
        }

        @Override
        public String textualFormat()
        {
            List<String> supportedCharsets = new ArrayList<>(Charset.availableCharsets().keySet());
            return String.join(", ",supportedCharsets);
        }
    });
}

图片

I checked the documentation of argparse4j lib and find no way to set case-insensitive choices, so an ArgumentChoice subclass is needed.

And for the big number of charsets, I think we can just put it aside bcuz how many charsets are supportted depends on the JVM itself.

qtc-de commented 7 months ago

Looks good to me :+1:

However, the pull request needs to be retargeted or recreated, as it does not reflect your latest changes on your develop branch :/