From de8ed6afaea42ae5e3eccaadf7b6c3c37cf5e0c3 Mon Sep 17 00:00:00 2001 From: uruwhy <58484522+uruwhy@users.noreply.github.com> Date: Wed, 8 Apr 2026 20:38:34 -0400 Subject: [PATCH 1/3] encrypt user-uploaded payloads and decrypt on read --- app/api/v2/handlers/payload_api.py | 44 ++++++++++---- app/service/file_svc.py | 13 +++- tests/api/v2/handlers/test_payloads_api.py | 69 +++++++++++++++++++++- 3 files changed, 114 insertions(+), 12 deletions(-) diff --git a/app/api/v2/handlers/payload_api.py b/app/api/v2/handlers/payload_api.py index 6c6350348..ff9ebc646 100644 --- a/app/api/v2/handlers/payload_api.py +++ b/app/api/v2/handlers/payload_api.py @@ -1,9 +1,11 @@ import asyncio import itertools +import logging import os import pathlib import re from io import IOBase +from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes from typing import Optional import aiohttp_apispec @@ -13,6 +15,9 @@ from app.api.v2.schemas.payload_schemas import PayloadQuerySchema, PayloadSchema, PayloadCreateRequestSchema, \ PayloadDeleteRequestSchema +USER_PAYLOAD_ENCRYPTION_FLAG = bytes('%userencryptedpayload%', encoding='utf-8') +PAYLOAD_API_LOGGER_NAME = 'payload_api_handler' + class PayloadApi(BaseApi): def __init__(self, services): @@ -87,7 +92,7 @@ async def post_payloads(self, request: web.Request): # Save the file to a temporary location first temp_file_path = pathlib.Path(file_path).parent / f"temp_{file_name}" loop: asyncio.AbstractEventLoop = asyncio.get_event_loop() - await loop.run_in_executor(None, self.__save_file, str(temp_file_path), file_field.file) + await loop.run_in_executor(None, self._save_file, str(temp_file_path), file_field.file) # Validate the saved file to ensure it is not a symbolic link if temp_file_path.is_symlink(): @@ -114,14 +119,14 @@ async def delete_payloads(self, request: web.Request): # Filename Input Validation if not file_name: - return web.HTTPBadRequest(reason="File name is required.") + raise web.HTTPBadRequest(reason="File name is required.") # Sanitize the filename sanitized_filename = self.sanitize_filename(file_name) # Additional safety checks if not sanitized_filename or sanitized_filename in ['.', '..']: - return web.HTTPBadRequest(reason="Invalid file name.") + raise web.HTTPBadRequest(reason="Invalid file name.") try: safe_path = self.validate_and_canonicalize_path(sanitized_filename) @@ -129,14 +134,13 @@ async def delete_payloads(self, request: web.Request): if safe_path_obj.is_symlink(): raise ValueError(f"Invalid path: {sanitized_filename} is a symbolic link.") os.remove(safe_path_obj) - response = web.HTTPNoContent() except ValueError as e: - response = web.HTTPNotFound(reason=str(e)) + raise web.HTTPNotFound(reason=str(e)) except FileNotFoundError: - response = web.HTTPNotFound() + raise web.HTTPNotFound() except PermissionError: - response = web.HTTPForbidden(reason="Permission denied.") - return response + raise web.HTTPForbidden(reason="Permission denied.") + raise web.HTTPNoContent() @classmethod async def __generate_file_name_and_path(cls, sanitized_filename: str) -> [str, str]: @@ -162,9 +166,15 @@ async def __generate_file_name_and_path(cls, sanitized_filename: str) -> [str, s return file_name, file_path @staticmethod - def __save_file(target_file_path: str, io_base_src: IOBase): + def _save_file(target_file_path: str, io_base_src: IOBase): """ Save an uploaded file content into a targeted file path. + To prevent unintended server-side execution of payloads, user-provided + payloads will be encrypted using a randomly generated key and IV. + + The on-disk file format is as follows: + USER_PAYLOAD_ENCRYPTION_FLAG + key + IV + ciphertext + Note this method calls blocking methods and must be run into a dedicated thread. :param target_file_path: The destination path to write to. @@ -172,14 +182,28 @@ def __save_file(target_file_path: str, io_base_src: IOBase): """ size: int = 0 read_chunk: bool = True + key = os.urandom(32) + iv = os.urandom(16) + cipher = Cipher(algorithms.AES(key), modes.CTR(iv)) + encryptor = cipher.encryptor() + with open(target_file_path, 'wb') as buffered_io_base_dest: + # Write flag, key, and IV prior to ciphertext. + buffered_io_base_dest.write(USER_PAYLOAD_ENCRYPTION_FLAG + key + iv) + + # Encrypt each chunk prior to appending it to the file + # We use CTR mode to take advantage of the stream cipher. while read_chunk: chunk: bytes = io_base_src.read(8192) if chunk: size += len(chunk) - buffered_io_base_dest.write(chunk) + buffered_io_base_dest.write(encryptor.update(chunk)) else: read_chunk = False + buffered_io_base_dest.write(encryptor.finalize()) + logging.getLogger(PAYLOAD_API_LOGGER_NAME).debug( + f'Encrypted {size} bytes of user payload and wrote to disk at {target_file_path}' + ) @staticmethod def validate_and_canonicalize_path(input_path: str, base_directory: str = "data/payloads/") -> str: diff --git a/app/service/file_svc.py b/app/service/file_svc.py index ae032b4a0..c661e09fc 100644 --- a/app/service/file_svc.py +++ b/app/service/file_svc.py @@ -13,8 +13,10 @@ from cryptography.fernet import Fernet, InvalidToken from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC +from app.api.v2.handlers.payload_api import USER_PAYLOAD_ENCRYPTION_FLAG from app.service.interfaces.i_file_svc import FileServiceInterface from app.utility.base_service import BaseService from app.utility.payload_encoder import xor_file, xor_bytes @@ -279,7 +281,16 @@ def _save(self, filename, content, encrypt=True): def _read(self, filename): with open(filename, 'rb') as f: buf = f.read() - if self.encryptor and buf.startswith(bytes(FILE_ENCRYPTION_FLAG, encoding='utf-8')): + if buf.startswith(USER_PAYLOAD_ENCRYPTION_FLAG): + # Handle encrypted user-uploaded payloads + buf = buf[len(USER_PAYLOAD_ENCRYPTION_FLAG):] + key = buf[0:32] + iv = buf[32:48] + ciphertext = buf[48:] + cipher = Cipher(algorithms.AES(key), modes.CTR(iv)) + decryptor = cipher.decryptor() + buf = decryptor.update(ciphertext) + decryptor.finalize() + elif self.encryptor and buf.startswith(bytes(FILE_ENCRYPTION_FLAG, encoding='utf-8')): try: buf = self.encryptor.decrypt(buf[len(FILE_ENCRYPTION_FLAG):]) except InvalidToken: diff --git a/tests/api/v2/handlers/test_payloads_api.py b/tests/api/v2/handlers/test_payloads_api.py index 4ffa34eb7..e8fdf3600 100644 --- a/tests/api/v2/handlers/test_payloads_api.py +++ b/tests/api/v2/handlers/test_payloads_api.py @@ -1,7 +1,13 @@ +import io import os import pathlib import tempfile +from aiohttp import FormData from http import HTTPStatus +from unittest import mock + +from app.api.v2.handlers.payload_api import PayloadApi +from app.utility.base_service import BaseService import pytest @@ -36,8 +42,12 @@ def expected_payload_file_names(expected_payload_file_paths): return {os.path.basename(path) for path in expected_payload_file_paths} -class TestPayloadsApi: +# Return n 0x01 bytes +def _mock_urandom(n): + return b'\x01' * n + +class TestPayloadsApi: async def test_get_payloads(self, api_v2_client, api_cookies, expected_payload_file_names): resp = await api_v2_client.get('/api/v2/payloads', cookies=api_cookies) payload_file_names = await resp.json() @@ -82,3 +92,60 @@ async def test_get_payloads_name_filter_with_sort_and_add_path( async def test_unauthorized_get_payloads(self, api_v2_client): resp = await api_v2_client.get('/api/v2/payloads') assert resp.status == HTTPStatus.UNAUTHORIZED + + @mock.patch.object(pathlib.Path, 'rename') + async def test_post_payloads(self, mock_rename, api_v2_client, api_cookies): + file_data = bytes([0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef]) + header_data = b'%userencryptedpayload%' + (b'\x01' * 48) # 32-byte key + 16-byte IV + ciphertext = bytes([0x9d, 0x8f, 0xd1, 0xa1, 0x3d, 0x2e, 0xac, 0x17]) + with tempfile.TemporaryFile(mode='w+b') as tmp_file: + tmp_file.write(file_data) + tmp_file.flush() + tmp_file.seek(0) + + m = mock.mock_open() + with mock.patch.object(os, 'urandom', wraps=_mock_urandom): + with mock.patch('builtins.open', m): + upload_data = FormData() + upload_data.add_field('file', tmp_file, filename='testpostpayload') + resp = await api_v2_client.post('/api/v2/payloads', + data=upload_data) + assert resp.status == HTTPStatus.OK + assert await resp.json() == dict(payloads=['testpostpayload']) + mock_rename.assert_called_with('data/payloads/testpostpayload') + m.assert_called_with('data/payloads/temp_testpostpayload', 'wb') + m().write.assert_any_call(header_data) + m().write.assert_any_call(ciphertext) + m().write.assert_called_with(b'') + + def test_save_file(self): + original_data = os.urandom(24*1024) + with tempfile.NamedTemporaryFile(delete_on_close=False) as fp: + PayloadApi._save_file(fp.name, io.BytesIO(original_data)) + decrypted = BaseService.get_service('file_svc')._read(fp.name) + assert decrypted == original_data + + async def test_delete_payloads(self, api_v2_client, api_cookies): + want_path = pathlib.Path('data/payloads/testtodelete').resolve() + with mock.patch.object(os, 'remove') as mock_remove: + resp = await api_v2_client.delete('/api/v2/payloads/testtodelete') + mock_remove.assert_called_once_with(want_path) + assert resp.status == 204 + + # Test ValueError + with mock.patch.object(os, 'remove', side_effect=ValueError('testvalueerror')) as mock_remove: + resp = await api_v2_client.delete('/api/v2/payloads/testtodelete') + assert resp.status == 404 + assert resp.reason == 'testvalueerror' + + # Test FileNotFoundError + with mock.patch.object(os, 'remove', side_effect=FileNotFoundError()) as mock_remove: + resp = await api_v2_client.delete('/api/v2/payloads/testtodelete') + assert resp.status == 404 + assert resp.reason == 'Not Found' + + # Test PermissionError + with mock.patch.object(os, 'remove', side_effect=PermissionError()) as mock_remove: + resp = await api_v2_client.delete('/api/v2/payloads/testtodelete') + assert resp.status == 403 + assert resp.reason == 'Permission denied.' From 0434ce9c07f92d8751545d59b0351675d89718b7 Mon Sep 17 00:00:00 2001 From: uruwhy <58484522+uruwhy@users.noreply.github.com> Date: Wed, 8 Apr 2026 21:53:57 -0400 Subject: [PATCH 2/3] make test compatible with earlier versions of python --- tests/api/v2/handlers/test_payloads_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/api/v2/handlers/test_payloads_api.py b/tests/api/v2/handlers/test_payloads_api.py index e8fdf3600..3b1c1fb66 100644 --- a/tests/api/v2/handlers/test_payloads_api.py +++ b/tests/api/v2/handlers/test_payloads_api.py @@ -120,7 +120,7 @@ async def test_post_payloads(self, mock_rename, api_v2_client, api_cookies): def test_save_file(self): original_data = os.urandom(24*1024) - with tempfile.NamedTemporaryFile(delete_on_close=False) as fp: + with tempfile.NamedTemporaryFile() as fp: PayloadApi._save_file(fp.name, io.BytesIO(original_data)) decrypted = BaseService.get_service('file_svc')._read(fp.name) assert decrypted == original_data From 60d8a4bbe65189f40bdbbc0d131aa7b5cc2783db Mon Sep 17 00:00:00 2001 From: uruwhy <58484522+uruwhy@users.noreply.github.com> Date: Tue, 19 May 2026 10:23:53 -0400 Subject: [PATCH 3/3] pr review suggestions --- app/api/v2/handlers/payload_api.py | 15 ++++++++------- app/service/file_svc.py | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/api/v2/handlers/payload_api.py b/app/api/v2/handlers/payload_api.py index ff9ebc646..ffd94fa45 100644 --- a/app/api/v2/handlers/payload_api.py +++ b/app/api/v2/handlers/payload_api.py @@ -14,8 +14,8 @@ from app.api.v2.handlers.base_api import BaseApi from app.api.v2.schemas.payload_schemas import PayloadQuerySchema, PayloadSchema, PayloadCreateRequestSchema, \ PayloadDeleteRequestSchema +from app.service.file_svc import USER_PAYLOAD_ENCRYPTION_FLAG -USER_PAYLOAD_ENCRYPTION_FLAG = bytes('%userencryptedpayload%', encoding='utf-8') PAYLOAD_API_LOGGER_NAME = 'payload_api_handler' @@ -119,14 +119,14 @@ async def delete_payloads(self, request: web.Request): # Filename Input Validation if not file_name: - raise web.HTTPBadRequest(reason="File name is required.") + return web.HTTPBadRequest(reason="File name is required.") # Sanitize the filename sanitized_filename = self.sanitize_filename(file_name) # Additional safety checks if not sanitized_filename or sanitized_filename in ['.', '..']: - raise web.HTTPBadRequest(reason="Invalid file name.") + return web.HTTPBadRequest(reason="Invalid file name.") try: safe_path = self.validate_and_canonicalize_path(sanitized_filename) @@ -134,13 +134,14 @@ async def delete_payloads(self, request: web.Request): if safe_path_obj.is_symlink(): raise ValueError(f"Invalid path: {sanitized_filename} is a symbolic link.") os.remove(safe_path_obj) + response = web.HTTPNoContent() except ValueError as e: - raise web.HTTPNotFound(reason=str(e)) + response = web.HTTPNotFound(reason=str(e)) except FileNotFoundError: - raise web.HTTPNotFound() + response = web.HTTPNotFound() except PermissionError: - raise web.HTTPForbidden(reason="Permission denied.") - raise web.HTTPNoContent() + response = web.HTTPForbidden(reason="Permission denied.") + return response @classmethod async def __generate_file_name_and_path(cls, sanitized_filename: str) -> [str, str]: diff --git a/app/service/file_svc.py b/app/service/file_svc.py index 2474d1fa7..a141d61c5 100644 --- a/app/service/file_svc.py +++ b/app/service/file_svc.py @@ -16,7 +16,6 @@ from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC -from app.api.v2.handlers.payload_api import USER_PAYLOAD_ENCRYPTION_FLAG from app.service.interfaces.i_file_svc import FileServiceInterface from app.utility.base_service import BaseService from app.utility.payload_encoder import xor_file, xor_bytes @@ -30,6 +29,7 @@ 'http': URL_SANITIZATION_REGEX, 'socket': re.compile(r'^[\w\-\.:]+$') } +USER_PAYLOAD_ENCRYPTION_FLAG = bytes('%userencryptedpayload%', encoding='utf-8') class FileSvc(FileServiceInterface, BaseService):