Home directory prefix support

[WesVleuten]

Description of Changes

Ran into an issue where upload ~/file.txt /tmp/file.txt errored out on me. Looking into this, I found pwncat doesn't support the home directory prefix.

This is just a possible fix I've come up with, I'd love to hear feedback on these changes and if you think I'm heading into the right direction.

The way I've currently done this would also be able to allow for other prefixes to be configured for pwncat at some point. I can imagine wanting to have a prefix like @ for where I've stored my static binaries.

Major Changes Implemented:

• Introduces parameter parsers
• Added home directory (~) support

Pre-Merge Tasks

• Formatted all modified files w/ python-black
• Sorted imports for modified files w/ isort
• Ran flake8 on repo, and fixed any new problems w/ modified files
• Ran pytest test cases
• Added brief summary of updates to CHANGELOG (under [Unreleased])

[Mitul16]

download module seems to have the same issue.
If we are downloading a file to be saved in ~/... then ~ directory is not parsed to be replaced with $HOME

Would you like to implement that as well?

[WesVleuten]

Thanks for the feedback! It now also is implemented for the remote filesystem, with windows support, and has been added to the download command.

Maybe it's a good idea to combine ParameterParsers and Completers into a ParameterType?

[calebstewart]

I'm crazy late to this conversation, but trying to clean up here. I've been away from this project for a bit.

I have a couple implementation comments. We shouldn't need to do the testing for tilde's and such manually. There are routines to do this already.

• In the case of local files, os.path.expanduser will do nothing if there isn't a tilde. We can call it regardless, which falls back to the OS routines to do parsing. This is ideal.
• Similarly, we can use session.platform.Path(value).expanduser() so we don't reinvent that wheel. The generic implementation handles ~/path/to/file and ~username/path/to/file and is at pwncat/platform/__init__.py:153.

Regarding the ParamType stuff, I think this is kind of re-inventing some argparse options that we could use. We can do a similar thing, but with the type argument to the argparse object. Currently, there's a weird syntax for that, but I can update it. Doing this technically works right now:

class Command(CommandDefinition):

  def expand_remote(self, path):
    return self.manager.target.platform.Path(path).expanduser()

  ARGS = {
    "test": Parameter(Complete.REMOTE_FILE, type=("method", expand_remote))
  }

Basically, pwncat passing a majority of the arguments to Parameter on directly to the add_argument method from argparse. However, in the case of type, it will wrap your method so that it receives a reference to self if you assign it in that way.

There's two things wrong with that:

1. It's cumbersome (the ("method", function) syntax is awkward.
2. It's a lot of boilerplate.

Those two problems are pretty easy to solve. First, I can change the type argument to always be a callable that accepts self. I did that for choices and it has worked very well, so I don't see why it wouldn't work here. Second, we can create similar helpers to what you did, but just a little simpler. E.g.

# Defined in `pwncat/commands/__init__.py` for all commands to use
def RemotePathType(parser, path):
  return parser.manager.target.platform.Path(path).expanduser()

def LocalPathType(parser, path):
  return pathlib.Path(path).expanduser()

# Example of usage
class Command(CommandDefinition):

  ARGS = {
    "test": Parameter(Complete.REMOTE_FILE, type=RemoteFileType)
  }

The upside with the above implementation is that the resulting args.test variable would be a path-like object automatically, which is kind of nice for implementing commands that work with paths. We could even go so far as specially interpreting Complete.REMOTE_FILE and Complete.LOCAL_FILE to automatically assign the type argument, but I'm not sure if that's a good idea. It feels kind of like "black magic" that will confuse people.

I'm going to work with this a bit on my end to see which solution I personally like the best. I'm still not sure if I want the paths to be automatically expanded. In the case of downloading/uploading files, that's true, but it may not always be true, and may cause problems if paths are mutated in the backend of the code without the user knowing what's happening.