Merge pull request #246 from Mitul16/master

Fixes #245 and minor patches

Author: calebstewart

CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 The Changelog starts with v0.4.1, because we did not keep one before that,
 and simply didn't have the time to go back and retroactively create one.
 
+## [Unreleased]
+### Fixed
+- Fixed `shlex.join` use with non-str type objects (e.g. `RemotePath`)
+- Fixed `set` command use with incorrect keys (e.g. `set invalid value`)
+
+### Added
+- Added missed `PlatformError` for `upload` command (e.g. "no gtfobins writers available")
+
 ## [0.5.4] - 2022-01-27
 Bug fix for the `load` command.
 

docs/source/conf.py

@@ -12,7 +12,8 @@
 #
 import os
 import sys
-sys.path.insert(0, os.path.abspath('../..'))
+
+sys.path.insert(0, os.path.abspath("../.."))
 
 
 # -- Project information -----------------------------------------------------

pwncat/commands/run.py

@@ -68,12 +68,6 @@ def run(self, manager: "pwncat.manager.Manager", args):
 
             if args.module is not None:
                 manager.config.back()
-        except pwncat.modules.ModuleFailed as exc:
-            if args.traceback:
-                console.print_exception()
-            else:
-                console.log(f"[red]error[/red]: module failed: {exc}")
-            return
         except pwncat.modules.ModuleNotFound:
             console.log(f"[red]error[/red]: {module_name}: not found")
             return
@@ -86,6 +80,12 @@ def run(self, manager: "pwncat.manager.Manager", args):
         except pwncat.modules.InvalidArgument as exc:
             console.log(f"[red]error[/red]: invalid argument: {exc}")
             return
+        except pwncat.modules.ModuleFailed as exc:
+            if args.traceback:
+                console.print_exception()
+            else:
+                console.log(f"[red]error[/red]: module failed: {exc}")
+            return
 
         if isinstance(result, list):
             result = [r for r in result if not r.hidden]

pwncat/commands/set.py

@@ -80,9 +80,14 @@ def run(self, manager, args):
                 try:
                     if manager.sessions and args.variable == "db":
                         raise ValueError("cannot change database with running session")
-                    manager.config.set(
-                        args.variable, args.value, getattr(args, "global")
-                    )
+                    if args.variable in manager.config:
+                        manager.config.set(
+                            args.variable, args.value, getattr(args, "global")
+                        )
+                    else:
+                        console.log(
+                            f"[red]error[/red]: invalid choice {repr(args.variable)}"
+                        )
                     if args.variable == "db":
                         # Ensure the database is re-opened, if it was already
                         manager.open_database()
@@ -95,10 +100,15 @@ def run(self, manager, args):
                 except ValueError as exc:
                     console.log(f"[red]error[/red]: {exc}")
             elif args.variable is not None:
-                value = manager.config[args.variable]
-                console.print(
-                    f" [cyan]{args.variable}[/cyan] = [yellow]{repr(value)}[/yellow]"
-                )
+                if args.variable in manager.config:
+                    value = manager.config[args.variable]
+                    console.print(
+                        f" [cyan]{args.variable}[/cyan] = [yellow]{repr(value)}[/yellow]"
+                    )
+                else:
+                    console.log(
+                        f"[red]error[/red]: invalid choice {repr(args.variable)}"
+                    )
             else:
                 for name in manager.config:
                     value = manager.config[name]

pwncat/commands/upload.py

@@ -14,6 +14,7 @@
 import pwncat
 from pwncat.util import console, copyfileobj, human_readable_size, human_readable_delta
 from pwncat.commands import Complete, Parameter, CommandDefinition
+from pwncat.platform import PlatformError
 
 
 class Command(CommandDefinition):
@@ -77,5 +78,10 @@ def run(self, manager: "pwncat.manager.Manager", args):
                 f"uploaded [cyan]{human_readable_size(length)}[/cyan] "
                 f"in [green]{human_readable_delta(elapsed)}[/green]"
             )
-        except (FileNotFoundError, PermissionError, IsADirectoryError) as exc:
+        except (
+            FileNotFoundError,
+            PermissionError,
+            IsADirectoryError,
+            PlatformError,
+        ) as exc:
             self.parser.error(str(exc))

pwncat/modules/linux/implant/authorized_key.py

@@ -41,7 +41,7 @@ def remove(self, session: "pwncat.manager.Session"):
         user = session.find_user(uid=self.uid)
 
         if current_user.id != self.uid and current_user.id != 0:
-            raise ModuleFailed(f"must be root or {user.name}")
+            raise ModuleFailed(f"must be [blue]root[/blue] or [blue]{user.name}[/blue]")
 
         # Ensure the directory exists
         homedir = session.platform.Path(user.home)
@@ -93,10 +93,12 @@ def install(self, session: "pwncat.manager.Session", user, key):
         yield Status("verifying user permissions")
         current_user = session.current_user()
         if user != "__pwncat_current__" and current_user.id != 0:
-            raise ModuleFailed("only root can install implants for other users")
+            raise ModuleFailed(
+                "only [blue]root[/blue] can install implants for other users"
+            )
 
         if not os.path.isfile(key):
-            raise ModuleFailed(f"private key {key} does not exist")
+            raise ModuleFailed(f"private key [blue]{key}[/blue] does not exist")
 
         try:
             yield Status("reading public key")
@@ -119,7 +121,7 @@ def install(self, session: "pwncat.manager.Session", user, key):
         for implant in session.run("enumerate", types=["implant.*"]):
             if implant.source == self.name and implant.uid == user_info.uid:
                 raise ModuleFailed(
-                    f"{self.name} already installed for {user_info.name}"
+                    f"[blue]{self.name}[/blue] already installed for [blue]{user_info.name}[/blue]"
                 )
 
         # Ensure the directory exists

pwncat/platform/linux.py

@@ -1111,7 +1111,7 @@ def Popen(
             )
 
         if isinstance(args, list):
-            command = shlex.join(args)
+            command = shlex.join(str(arg) for arg in args)
         elif isinstance(args, str):
             command = args
         else:

pwncat/util.py

@@ -146,7 +146,7 @@ def human_readable_delta(seconds):
 
 
 def join(argv: List[str]):
-    """Join the string much line shlex.join, except assume that each token
+    """Join the string much like shlex.join, except assume that each token
     is expecting double quotes. This allows variable references within the
     tokens."""
 

tests/conftest.py

@@ -8,9 +8,10 @@
 from io import StringIO
 
 import pytest
-from pwncat.channel import ChannelError
 from Crypto.PublicKey import RSA
 
+from pwncat.channel import ChannelError
+
 PLATFORM_MAP = {"ubuntu": "linux", "centos": "linux", "windows": "windows"}
 
 
@@ -37,13 +38,13 @@ def connection_details_for(name):
 
 @pytest.fixture(params=["ubuntu", "centos"])
 def linux_details(request):
-    """ Get available connection details for linux hosts """
+    """Get available connection details for linux hosts"""
     return connection_details_for(request.param)
 
 
 @pytest.fixture(params=["windows"])
 def windows_details(request):
-    """ Get available connection details for windows hosts """
+    """Get available connection details for windows hosts"""
     return connection_details_for(request.param)
 
 
@@ -84,18 +85,18 @@ def session_for(request):
 
 @pytest.fixture(params=["windows", "ubuntu", "centos"])
 def session(request):
-    """ Start a session with any platform """
+    """Start a session with any platform"""
     yield from session_for(request)
 
 
 @pytest.fixture(params=["windows"])
 def windows(request):
-    """ Start a windows session """
+    """Start a windows session"""
     yield from session_for(request)
 
 
 @pytest.fixture(params=["ubuntu", "centos"])
 def linux(request):
-    """ Start a linux session """
+    """Start a linux session"""
 
     yield from session_for(request)

tests/test_session.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 
 import pytest
+
 from pwncat.modules import IncorrectPlatformError
 
 
@@ -13,13 +14,13 @@ def test_session_iter_users(session):
 
 
 def test_session_find_user_name(session):
-    """ Test that locating a user by name works """
+    """Test that locating a user by name works"""
 
     assert session.find_user(name="john") is not None
 
 
 def test_session_find_user_uid(linux):
-    """ Test locating a user by their UID (for linux only) """
+    """Test locating a user by their UID (for linux only)"""
 
     user = linux.find_user(uid=0)
 
@@ -28,7 +29,7 @@ def test_session_find_user_uid(linux):
 
 
 def test_session_find_user_sid(windows):
-    """ Test locating a user by their SID (for windows only) """
+    """Test locating a user by their SID (for windows only)"""
 
     # This is the SID of the Administrator in the windows servercore image...
     # This will only work from the testing container, but I've decided that's fine.
@@ -39,30 +40,30 @@ def test_session_find_user_sid(windows):
 
 
 def test_session_find_module(session):
-    """ Test that locating modules works """
+    """Test that locating modules works"""
 
     assert len(list(session.find_module("enumerate.*"))) > 0
     assert len(list(session.find_module("enumerate.user"))) == 1
     assert len(list(session.find_module("module_does_not_exist"))) == 0
 
 
 def test_session_run_module(session):
-    """ Test running a module within a session """
+    """Test running a module within a session"""
 
     # We should be able to enumerate a hostname
     facts = session.run("enumerate", types=["system.hostname"])
     assert len(facts) > 0
 
 
 def test_session_wrong_platform_linux(linux):
-    """ Test that windows modules don't run in linux """
+    """Test that windows modules don't run in linux"""
 
     with pytest.raises(IncorrectPlatformError):
         linux.run("windows.enumerate.user")
 
 
 def test_session_wrong_platform_windows(windows):
-    """ Test that linux modules don't run on windows """
+    """Test that linux modules don't run on windows"""
 
     with pytest.raises(IncorrectPlatformError):
         windows.run("linux.enumerate.user")