From 8e40b1759d9f2e08da230e097cb623d6db87a51a Mon Sep 17 00:00:00 2001 From: Caleb Stewart Date: Thu, 17 Jun 2021 13:29:14 -0400 Subject: [PATCH 1/5] Removed C-d loop in favor of static C-d count It appears that you need to send every C-d twice, but I can't figure out why. All manual testing only requires a single C-d, but double each seems to correctly behave with file IO. --- pwncat/commands/upload.py | 11 ----------- pwncat/platform/linux.py | 35 ++++++++++++++++++----------------- pwncat/util.py | 4 ++-- 3 files changed, 20 insertions(+), 30 deletions(-) diff --git a/pwncat/commands/upload.py b/pwncat/commands/upload.py index a2d3dc0..13e547d 100644 --- a/pwncat/commands/upload.py +++ b/pwncat/commands/upload.py @@ -45,17 +45,6 @@ class Command(CommandDefinition): if not args.destination: args.destination = f"./{os.path.basename(args.source)}" - # else: - # access = pwncat.victim.access(args.destination) - # if Access.DIRECTORY in access: - # args.destination = os.path.join( - # args.destination, os.path.basename(args.source) - # ) - # elif Access.PARENT_EXIST not in access: - # console.log( - # f"[cyan]{args.destination}[/cyan]: no such file or directory" - # ) - # return try: length = os.path.getsize(args.source) diff --git a/pwncat/platform/linux.py b/pwncat/platform/linux.py index 7abb5c9..63a6300 100644 --- a/pwncat/platform/linux.py +++ b/pwncat/platform/linux.py @@ -420,7 +420,7 @@ class LinuxWriter(BufferedIOBase): for idx, c in enumerate(b): # Track when the last new line was - if c == 0x0D: + if c == 0x0A: self.since_newline = 0 else: self.since_newline += 1 @@ -432,7 +432,7 @@ class LinuxWriter(BufferedIOBase): # Track all characters in translated buffer translated.append(c) - if self.since_newline >= 4095: + if self.since_newline >= 2048: # Flush read immediately to prevent truncation of line translated.append(0x04) self.since_newline = 0 @@ -463,21 +463,22 @@ class LinuxWriter(BufferedIOBase): self.detach() return - # The number of C-d's needed to trigger an EOF in - # the process and exit is inconsistent based on the - # previous input. So, instead of trying to be deterministic, - # we simply send one and check. We do this until we find - # the ending delimeter and then exit. If the `on_close` - # hook was setup properly, this should be fine. - while True: - try: - self.popen.stdin.write(b"\x04") - self.popen.stdin.flush() - # Check for completion - self.popen.wait(timeout=0.1) - break - except pwncat.subprocess.TimeoutExpired: - continue + # Trigger EOF in remote process + self.popen.platform.channel.send(b"\x04") + self.popen.platform.channel.send(b"\x04") + if self.since_newline != 0: + self.popen.platform.channel.send(b"\x04") + self.popen.platform.channel.send(b"\x04") + + # Wait for the process to exit + try: + self.popen.wait() + except KeyboardInterrupt: + # We *should* wait for the process to send the return codes. + # however, if the user wants to stop this process, we should + # at least attempt to do whatever cleanup we can. + self.popen.kill() + self.popen.wait() # Ensure we don't touch stdio again self.detach() diff --git a/pwncat/util.py b/pwncat/util.py index 1d8ec53..b07f3a6 100644 --- a/pwncat/util.py +++ b/pwncat/util.py @@ -118,9 +118,9 @@ def isprintable(data) -> bool: def human_readable_size(size, decimal_places=2): for unit in ["B", "KiB", "MiB", "GiB", "TiB"]: - if size < 1024.0: + if size < 1000.0: return f"{size:.{decimal_places}f}{unit}" - size /= 1024.0 + size /= 1000.0 return f"{size:.{decimal_places}f}{unit}" From 5544954852945f55091964afdb2f0d511197f413 Mon Sep 17 00:00:00 2001 From: Caleb Stewart Date: Fri, 18 Jun 2021 14:10:56 -0400 Subject: [PATCH 2/5] Added better file io tests which pass --- tests/test_fileio.py | 49 ++++++++++++++++++++++++++++++++++++++++++ tests/test_platform.py | 27 ++++------------------- 2 files changed, 53 insertions(+), 23 deletions(-) create mode 100644 tests/test_fileio.py diff --git a/tests/test_fileio.py b/tests/test_fileio.py new file mode 100644 index 0000000..e1b40a8 --- /dev/null +++ b/tests/test_fileio.py @@ -0,0 +1,49 @@ +#!/usr/bin/env python3 + +from pwncat.util import random_string + + +def do_file_test(session, content): + """Do a generic file test""" + + name = random_string() + ".txt" + mode = "b" if isinstance(content, bytes) else "" + + with session.platform.open(name, mode + "w") as filp: + assert filp.write(content) == len(content) + + with session.platform.open(name, mode + "r") as filp: + assert filp.read() == content + + # In some cases, the act of reading/writing causes a shell to hang + # so double check that. + result = session.platform.run( + ["echo", "hello world"], capture_output=True, text=True + ) + assert result.stdout == "hello world\n" + + +def test_small_text(session): + """Test writing a small text-only file""" + + do_file_test(session, "hello world") + + +def test_large_text(session): + """Test writing and reading a large text file""" + + contents = ("A" * 1000 + "\n") * 10 + do_file_test(session, contents) + + +def test_small_binary(session): + """Test writing a small amount of binary data""" + + contents = bytes(list(range(32))) + do_file_test(session, contents) + + +def test_large_binary(session): + + contents = bytes(list(range(32))) * 400 + do_file_test(session, contents) diff --git a/tests/test_platform.py b/tests/test_platform.py index cc1ddcc..c81525e 100644 --- a/tests/test_platform.py +++ b/tests/test_platform.py @@ -10,27 +10,8 @@ from pwncat.util import random_string from pwncat.platform.windows import PowershellError -def test_platform_file_io(session): - """ Test file read/write of printable data """ - - # Generate random binary data - contents = os.urandom(1024) - - # Create a new temporary file - with session.platform.tempfile(mode="wb") as filp: - filp.write(contents) - path = filp.name - - # Ensure it exists - assert session.platform.Path(path).exists() - - # Read the data back and ensure it matches - with session.platform.open(path, "rb") as filp: - assert contents == filp.read() - - def test_platform_dir_io(session): - """ Test creating a directory and interacting with the contents """ + """Test creating a directory and interacting with the contents""" # Create a path object representing the new remote directory path = session.platform.Path(random_string()) @@ -61,7 +42,7 @@ def test_platform_run(session): def test_platform_su(session): - """ Test running `su` """ + """Test running `su`""" try: session.platform.su("john", "P@ssw0rd") @@ -77,7 +58,7 @@ def test_platform_su(session): def test_platform_sudo(session): - """ Testing running `sudo` """ + """Testing running `sudo`""" try: @@ -103,7 +84,7 @@ def test_platform_sudo(session): def test_windows_powershell(windows): - """ Test powershell execution """ + """Test powershell execution""" # Run a real powershell snippet r = windows.platform.powershell("$PSVersionTable.PSVersion") From 521550dc82dd0907ce70ce91cef2f20e1e25b709 Mon Sep 17 00:00:00 2001 From: Caleb Stewart Date: Fri, 18 Jun 2021 14:12:47 -0400 Subject: [PATCH 3/5] Added CHANGELOG entries for PR141 --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a76753f..bf436e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and simply didn't have the time to go back and retroactively create one. ### Changed - Changed session tracking so session IDs aren't reused - Changed zsh prompt to match CWD of other shell prompts +- Changed LinuxWriter close routine again to account for needed EOF signals ([#140](https://github.com/calebstewart/pwncat/issues/140)) +### Added +- Added better file io test cases ## [0.4.2] - 2021-06-15 Quick patch release due to corrected bug in `ChannelFile` which caused command From 6c26df12c111a3cd350bb0cedeb04a2daf19945e Mon Sep 17 00:00:00 2001 From: Caleb Stewart Date: Fri, 18 Jun 2021 18:30:57 -0400 Subject: [PATCH 4/5] Re-added readline import after regression --- pwncat/platform/windows.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pwncat/platform/windows.py b/pwncat/platform/windows.py index 2221bf8..dfcf78e 100644 --- a/pwncat/platform/windows.py +++ b/pwncat/platform/windows.py @@ -26,6 +26,7 @@ import hashlib import pathlib import tarfile import binascii +import readline # noqa: F401 import functools import threading import subprocess From b680c1f276884f38417b96a9ead6d55147ce6459 Mon Sep 17 00:00:00 2001 From: Caleb Stewart Date: Fri, 18 Jun 2021 18:33:21 -0400 Subject: [PATCH 5/5] Added changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf436e4..4c30cfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and simply didn't have the time to go back and retroactively create one. ### Fixed - Pinned container base image to alpine 3.13.5 and installed to virtualenv ([#134](https://github.com/calebstewart/pwncat/issues/134)) - Fixed syntax for f-strings in escalation command +- Re-added `readline` import for windows platform after being accidentally removed ### Changed - Changed session tracking so session IDs aren't reused - Changed zsh prompt to match CWD of other shell prompts