From ba6d0a8a10af687393134fc1e9662100ce67df52 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 19 Nov 2022 21:10:20 -0500 Subject: Implement files request form --- tests/api/test_eval.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'tests/api/test_eval.py') diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index 976970e..caa848e 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -5,7 +5,7 @@ class TestEvalResource(SnekAPITestCase): PATH = "/eval" def test_post_valid_200(self): - body = {"input": "foo"} + body = {"args": ["-c", "print('output')"]} result = self.simulate_post(self.PATH, json=body) self.assertEqual(result.status_code, 200) @@ -20,26 +20,25 @@ class TestEvalResource(SnekAPITestCase): expected = { "title": "Request data failed validation", - "description": "'input' is a required property", + "description": "'args' is a required property", } self.assertEqual(expected, result.json) def test_post_invalid_data_400(self): - bodies = ({"input": 400}, {"input": "", "args": [400]}) - - for body in bodies: + bodies = ({"args": 400}, {"args": [], "files": [215]}) + expects = ["400 is not of type 'array'", "215 is not of type 'object'"] + for body, expected in zip(bodies, expects): with self.subTest(): result = self.simulate_post(self.PATH, json=body) self.assertEqual(result.status_code, 400) - expected = { + expected_json = { "title": "Request data failed validation", - "description": "400 is not of type 'string'", + "description": expected, } - - self.assertEqual(expected, result.json) + self.assertEqual(expected_json, result.json) def test_post_invalid_content_type_415(self): body = "{'input': 'foo'}" -- cgit v1.2.3 From 2ccf7e57ca817d15eae872a10aeef2120ab9f6c8 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Tue, 22 Nov 2022 14:13:46 -0500 Subject: Add addition path parse unit tests --- tests/api/test_eval.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) (limited to 'tests/api/test_eval.py') diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index caa848e..259cf0d 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -40,6 +40,58 @@ class TestEvalResource(SnekAPITestCase): } self.assertEqual(expected_json, result.json) + def test_files_path(self): + """Normal paths, should work with 200.""" + test_paths = [ + "file.txt", + "./file.jpg", + "path/to/file", + "folder/./to/./somewhere", + ] + for path in test_paths: + with self.subTest(path=path): + body = {"args": ["test.py"], "files": [{"path": path}]} + result = self.simulate_post(self.PATH, json=body) + self.assertEqual(result.status_code, 200) + self.assertEqual("output", result.json["stdout"]) + self.assertEqual(0, result.json["returncode"]) + + def test_files_illegal_path_traversal(self): + """Traversal beyond root, should be denied with 400 error.""" + test_paths = [ + "../secrets", + "dir/../../secrets", + "folder/./hm", + ] + for path in test_paths: + with self.subTest(path=path): + body = {"args": ["test.py"], "files": [{"path": path}]} + result = self.simulate_post(self.PATH, json=body) + self.assertEqual(result.status_code, 400) + expected = { + "title": "Request file path failed validation", + "description": f"File path '{path}' may not traverse beyond root", + } + self.assertEqual(expected, result.json) + + def test_files_illegal_path_absolute(self): + """Absolute file paths, should be denied with 400 error.""" + test_paths = [ + "/etc/vars/secrets", + "/absolute", + "/file.bin", + ] + for path in test_paths: + with self.subTest(path=path): + body = {"args": ["test.py"], "files": [{"path": path}]} + result = self.simulate_post(self.PATH, json=body) + self.assertEqual(result.status_code, 400) + expected = { + "title": "Request file path failed validation", + "description": f"File path '{path}' must be relative", + } + self.assertEqual(expected, result.json) + def test_post_invalid_content_type_415(self): body = "{'input': 'foo'}" headers = {"Content-Type": "application/xml"} -- cgit v1.2.3 From 00b7e3bd13b7914a44c5af00ff6f8da9b48fa9c5 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Tue, 22 Nov 2022 14:20:17 -0500 Subject: Fix path parse unit tests --- tests/api/test_eval.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'tests/api/test_eval.py') diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index 259cf0d..65cd9a4 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -46,7 +46,9 @@ class TestEvalResource(SnekAPITestCase): "file.txt", "./file.jpg", "path/to/file", + "folder/../hm", "folder/./to/./somewhere", + "traversal/but/../not/beyond/../root", ] for path in test_paths: with self.subTest(path=path): @@ -60,8 +62,9 @@ class TestEvalResource(SnekAPITestCase): """Traversal beyond root, should be denied with 400 error.""" test_paths = [ "../secrets", + "../../dir", "dir/../../secrets", - "folder/./hm", + "dir/var/../../../file", ] for path in test_paths: with self.subTest(path=path): -- cgit v1.2.3 From d67b70bdbb3277babd130dd4914f16de8ba8a9b5 Mon Sep 17 00:00:00 2001 From: Ionite Date: Mon, 28 Nov 2022 09:13:43 +0800 Subject: Docstring phrasing update Co-authored-by: Mark <1515135+MarkKoz@users.noreply.github.com> --- snekbox/nsjail.py | 4 ++-- snekbox/snekio.py | 4 ++-- tests/api/test_eval.py | 6 +++--- tests/test_libmount.py | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) (limited to 'tests/api/test_eval.py') diff --git a/snekbox/nsjail.py b/snekbox/nsjail.py index 4a0f45c..fcdd259 100644 --- a/snekbox/nsjail.py +++ b/snekbox/nsjail.py @@ -28,7 +28,7 @@ LOG_PATTERN = re.compile( def iter_lstrip(iterable: Iterable[_T]) -> Generator[_T, None, None]: - """Removes leading falsy objects from an iterable.""" + """Remove leading falsy objects from an iterable.""" it = iter(iterable) for item in it: if item: @@ -64,7 +64,7 @@ class NsJail: max_output_size: Maximum size of the output in bytes. read_chunk_size: Size of the read buffer in bytes. memfs_instance_size: Size of the tmpfs instance in bytes. - files_limit: Maximum number of files to parse for attach. + files_limit: Maximum number of files to parse for attachments. files_timeout: Maximum time in seconds to wait for files to be written / read. files_pattern: Pattern to match files to attach. """ diff --git a/snekbox/snekio.py b/snekbox/snekio.py index fd48585..5023a69 100644 --- a/snekbox/snekio.py +++ b/snekbox/snekio.py @@ -12,7 +12,7 @@ T = TypeVar("T", str, bytes) def safe_path(path: str) -> str: """ - Returns the `path` str if there are no security issues. + Return `path` if there are no security issues. Raises: IllegalPathError: Raised on any path rule violation. @@ -81,7 +81,7 @@ class FileAttachment(Generic[T]): return self.content.encode("utf-8") def save_to(self, directory: Path | str) -> None: - """Save the attachment to a path directory.""" + """Write the attachment to a file in `directory`.""" file = Path(directory, self.path) # Create directories if they don't exist file.parent.mkdir(parents=True, exist_ok=True) diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index 65cd9a4..c103880 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -41,7 +41,7 @@ class TestEvalResource(SnekAPITestCase): self.assertEqual(expected_json, result.json) def test_files_path(self): - """Normal paths, should work with 200.""" + """Normal paths should work with 200.""" test_paths = [ "file.txt", "./file.jpg", @@ -59,7 +59,7 @@ class TestEvalResource(SnekAPITestCase): self.assertEqual(0, result.json["returncode"]) def test_files_illegal_path_traversal(self): - """Traversal beyond root, should be denied with 400 error.""" + """Traversal beyond root should be denied with 400 error.""" test_paths = [ "../secrets", "../../dir", @@ -78,7 +78,7 @@ class TestEvalResource(SnekAPITestCase): self.assertEqual(expected, result.json) def test_files_illegal_path_absolute(self): - """Absolute file paths, should be denied with 400 error.""" + """Absolute file paths should be denied with 400 error.""" test_paths = [ "/etc/vars/secrets", "/absolute", diff --git a/tests/test_libmount.py b/tests/test_libmount.py index 8e56970..a9a5e5b 100644 --- a/tests/test_libmount.py +++ b/tests/test_libmount.py @@ -15,7 +15,7 @@ class LibMountTests(TestCase): @contextmanager def get_mount(self): - """Yields a valid mount point, unmounts after context.""" + """Yield a valid mount point and unmount after context.""" path = self.temp_dir / str(uuid4()) path.mkdir() try: -- cgit v1.2.3 From 7e7c0d6f5465f965b3ee31fc4f723ec23cc5ad94 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Mon, 28 Nov 2022 10:55:18 +0800 Subject: Add invalid, absolute, null byte paths to schema --- snekbox/api/resources/eval.py | 6 +++++- tests/api/test_eval.py | 11 +++++------ 2 files changed, 10 insertions(+), 7 deletions(-) (limited to 'tests/api/test_eval.py') diff --git a/snekbox/api/resources/eval.py b/snekbox/api/resources/eval.py index cedeb2e..764cc0b 100644 --- a/snekbox/api/resources/eval.py +++ b/snekbox/api/resources/eval.py @@ -33,7 +33,11 @@ class EvalResource: "items": { "type": "object", "properties": { - "path": {"type": "string"}, + "path": { + "type": "string", + # Disallow single forward slashes, absolute paths, and null bytes + "pattern": r"^[^/\\0].*", + }, "content": {"type": "string"}, }, "required": ["path"], diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index c103880..41bdd35 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -78,8 +78,10 @@ class TestEvalResource(SnekAPITestCase): self.assertEqual(expected, result.json) def test_files_illegal_path_absolute(self): - """Absolute file paths should be denied with 400 error.""" + """Absolute file paths should 400-error at json schema validation stage.""" test_paths = [ + "/", + "/etc", "/etc/vars/secrets", "/absolute", "/file.bin", @@ -89,11 +91,8 @@ class TestEvalResource(SnekAPITestCase): body = {"args": ["test.py"], "files": [{"path": path}]} result = self.simulate_post(self.PATH, json=body) self.assertEqual(result.status_code, 400) - expected = { - "title": "Request file path failed validation", - "description": f"File path '{path}' must be relative", - } - self.assertEqual(expected, result.json) + self.assertEqual("Request data failed validation", result.json["title"]) + self.assertIn("does not match", result.json["description"]) def test_post_invalid_content_type_415(self): body = "{'input': 'foo'}" -- cgit v1.2.3 From 0442a7cec24bff9985fb8cf59b0fb2604544eaa8 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Mon, 28 Nov 2022 11:12:56 +0800 Subject: Add ParsingError handling for invalid base64 --- snekbox/api/resources/eval.py | 10 ++++------ snekbox/snekio.py | 22 +++++++++++++--------- tests/api/test_eval.py | 2 +- 3 files changed, 18 insertions(+), 16 deletions(-) (limited to 'tests/api/test_eval.py') diff --git a/snekbox/api/resources/eval.py b/snekbox/api/resources/eval.py index 764cc0b..de54564 100644 --- a/snekbox/api/resources/eval.py +++ b/snekbox/api/resources/eval.py @@ -9,7 +9,7 @@ from snekbox.nsjail import NsJail __all__ = ("EvalResource",) -from snekbox.snekio import FileAttachment, IllegalPathError +from snekbox.snekio import FileAttachment, ParsingError log = logging.getLogger(__name__) @@ -35,7 +35,7 @@ class EvalResource: "properties": { "path": { "type": "string", - # Disallow single forward slashes, absolute paths, and null bytes + # Disallow single absolute paths, and null bytes "pattern": r"^[^/\\0].*", }, "content": {"type": "string"}, @@ -110,10 +110,8 @@ class EvalResource: py_args=req.media["args"], files=[FileAttachment.from_dict(file) for file in req.media.get("files", [])], ) - except IllegalPathError as e: - raise falcon.HTTPBadRequest( - title="Request file path failed validation", description=str(e) - ) + except ParsingError as e: + raise falcon.HTTPBadRequest(title="Request file is invalid", description=str(e)) except Exception: log.exception("An exception occurred while trying to process the request") raise falcon.HTTPInternalServerError diff --git a/snekbox/snekio.py b/snekbox/snekio.py index 2d2de6e..f57d3b0 100644 --- a/snekbox/snekio.py +++ b/snekbox/snekio.py @@ -28,16 +28,12 @@ def safe_path(path: str) -> str: return path -class AttachmentError(ValueError): - """Raised when an attachment is invalid.""" - - -class ParsingError(AttachmentError): - """Raised when an incoming file cannot be parsed.""" +class ParsingError(ValueError): + """Raised when an incoming content cannot be parsed.""" class IllegalPathError(ParsingError): - """Raised when an attachment has an illegal path.""" + """Raised when a request file has an illegal path.""" @dataclass(frozen=True) @@ -49,9 +45,17 @@ class FileAttachment: @classmethod def from_dict(cls, data: dict[str, str]) -> FileAttachment: - """Convert a dict to an attachment.""" + """ + Convert a dict to an attachment. + + Raises: + ParsingError: Raised when the dict has invalid base64 `content`. + """ path = safe_path(data["path"]) - content = b64decode(data.get("content", "")) + try: + content = b64decode(data.get("content", "")) + except (TypeError, ValueError) as e: + raise ParsingError(f"Invalid base64 encoding for file '{path}'") from e return cls(path, content) @classmethod diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index 41bdd35..64d7ba6 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -72,7 +72,7 @@ class TestEvalResource(SnekAPITestCase): result = self.simulate_post(self.PATH, json=body) self.assertEqual(result.status_code, 400) expected = { - "title": "Request file path failed validation", + "title": "Request file is invalid", "description": f"File path '{path}' may not traverse beyond root", } self.assertEqual(expected, result.json) -- cgit v1.2.3 From 84e73c12bb6e9b51c9e1bd4a7b6eae7ce65ed46e Mon Sep 17 00:00:00 2001 From: ionite34 Date: Mon, 28 Nov 2022 12:16:42 +0800 Subject: Add compat support for `input` arg --- snekbox/api/resources/eval.py | 23 ++++++++++++++++++++--- tests/api/test_eval.py | 18 +++++++++++------- 2 files changed, 31 insertions(+), 10 deletions(-) (limited to 'tests/api/test_eval.py') diff --git a/snekbox/api/resources/eval.py b/snekbox/api/resources/eval.py index de54564..c3e553a 100644 --- a/snekbox/api/resources/eval.py +++ b/snekbox/api/resources/eval.py @@ -27,6 +27,7 @@ class EvalResource: REQ_SCHEMA = { "type": "object", "properties": { + "input": {"type": "string"}, "args": {"type": "array", "items": {"type": "string"}}, "files": { "type": "array", @@ -44,7 +45,10 @@ class EvalResource: }, }, }, - "required": ["args"], + "anyOf": [ + {"required": ["input"]}, + {"required": ["args"]}, + ], } def __init__(self, nsjail: NsJail): @@ -57,6 +61,11 @@ class EvalResource: A list of arguments for the Python subprocess can be specified as `args`. + If `input` is specified, it will be appended as the last argument to `args`, + and `args` will have a default argument of `"-c"`. + + Either `input` or `args` must be specified. + The return codes mostly resemble those of a Unix shell. Some noteworthy cases: - None @@ -68,6 +77,10 @@ class EvalResource: Request body: + >>> { + ... "input": "print('Hello')" + ... } + >>> { ... "args": ["-c", "print('Hello')"] ... } @@ -105,10 +118,14 @@ class EvalResource: - 415 Unsupported content type; only application/JSON is supported """ + body: dict[str, str | list[str] | list[dict[str, str]]] = req.media + # If `input` is supplied, default `args` to `-c` + if "input" in body: + body.setdefault("args", ["-c"]) try: result = self.nsjail.python3( - py_args=req.media["args"], - files=[FileAttachment.from_dict(file) for file in req.media.get("files", [])], + py_args=body["args"], + files=[FileAttachment.from_dict(file) for file in body.get("files", [])], ) except ParsingError as e: raise falcon.HTTPBadRequest(title="Request file is invalid", description=str(e)) diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index 64d7ba6..26ac38a 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -5,12 +5,16 @@ class TestEvalResource(SnekAPITestCase): PATH = "/eval" def test_post_valid_200(self): - body = {"args": ["-c", "print('output')"]} - result = self.simulate_post(self.PATH, json=body) - - self.assertEqual(result.status_code, 200) - self.assertEqual("output", result.json["stdout"]) - self.assertEqual(0, result.json["returncode"]) + cases = [ + {"args": ["-c", "print('output')"]}, + {"input": "print('hello')"}, + ] + for body in cases: + with self.subTest(): + result = self.simulate_post(self.PATH, json=body) + self.assertEqual(result.status_code, 200) + self.assertEqual("output", result.json["stdout"]) + self.assertEqual(0, result.json["returncode"]) def test_post_invalid_schema_400(self): body = {"stuff": "foo"} @@ -20,7 +24,7 @@ class TestEvalResource(SnekAPITestCase): expected = { "title": "Request data failed validation", - "description": "'args' is a required property", + "description": "{'stuff': 'foo'} is not valid under any of the given schemas", } self.assertEqual(expected, result.json) -- cgit v1.2.3 From 95e3e18ac93709257840f8e5c36e6543678931f5 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sun, 4 Dec 2022 09:57:39 +0800 Subject: Add tests for test_eval 200 --- tests/api/test_eval.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'tests/api/test_eval.py') diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index 26ac38a..40369f5 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -8,6 +8,9 @@ class TestEvalResource(SnekAPITestCase): cases = [ {"args": ["-c", "print('output')"]}, {"input": "print('hello')"}, + {"input": "print('hello')", "args": ["-c"]}, + {"input": "print('hello')", "args": [""]}, + {"input": "pass", "args": ["-m", "timeit"]}, ] for body in cases: with self.subTest(): -- cgit v1.2.3 From 55a92231d83439b31441945799f713c25b569278 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sun, 4 Dec 2022 11:58:10 +0800 Subject: Add null byte schema validation tests --- tests/api/test_eval.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'tests/api/test_eval.py') diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index 40369f5..37f90e7 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -51,11 +51,14 @@ class TestEvalResource(SnekAPITestCase): """Normal paths should work with 200.""" test_paths = [ "file.txt", - "./file.jpg", + "./0.jpg", "path/to/file", "folder/../hm", "folder/./to/./somewhere", "traversal/but/../not/beyond/../root", + r"backslash\\okay", + r"backslash\okay", + "numbers/0123456789", ] for path in test_paths: with self.subTest(path=path): @@ -101,6 +104,23 @@ class TestEvalResource(SnekAPITestCase): self.assertEqual("Request data failed validation", result.json["title"]) self.assertIn("does not match", result.json["description"]) + def test_files_illegal_path_null_byte(self): + """Paths containing \0 should 400-error at json schema validation stage.""" + test_paths = [ + r"etc/passwd\0", + r"a\0b", + r"\0", + r"\\0", + r"var/\0/path", + ] + for path in test_paths: + with self.subTest(path=path): + body = {"args": ["test.py"], "files": [{"path": path}]} + result = self.simulate_post(self.PATH, json=body) + self.assertEqual(result.status_code, 400) + self.assertEqual("Request data failed validation", result.json["title"]) + self.assertIn("does not match", result.json["description"]) + def test_post_invalid_content_type_415(self): body = "{'input': 'foo'}" headers = {"Content-Type": "application/xml"} -- cgit v1.2.3