Skip to content

Reject control characters in decoded filenames #304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nitrobass24 merged 3 commits into from
Mar 23, 2026
Merged

Reject control characters in decoded filenames #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5d3a352
Reject control characters in decoded filenames (#300)
nitrobass24 Mar 19, 2026
5bfb18c
Assert queue_command not called on rejected filenames
nitrobass24 Mar 20, 2026
3054779
Fix pre-existing test failures and add assert_not_called checks
nitrobass24 Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 61 additions & 10 deletions src/python/tests/integration/test_web/test_handler/test_controller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ def side_effect(cmd: Controller.Command):
self.assertEqual(Controller.Command.Action.QUEUE, command.action)
self.assertEqual("test1", command.filename)

uri = quote(quote("/value/with/slashes", safe=""), safe="")
uri = quote(quote("value/with/slashes", safe=""), safe="")
print(self.test_app.get("/server/command/queue/" + uri))
command = self.controller.queue_command.call_args[0][0]
self.assertEqual(Controller.Command.Action.QUEUE, command.action)
self.assertEqual("/value/with/slashes", command.filename)
self.assertEqual("value/with/slashes", command.filename)

uri = quote(quote(" value with spaces", safe=""), safe="")
print(self.test_app.get("/server/command/queue/" + uri))
Expand Down Expand Up @@ -56,11 +56,11 @@ def side_effect(cmd: Controller.Command):
self.assertEqual(Controller.Command.Action.STOP, command.action)
self.assertEqual("test1", command.filename)

uri = quote(quote("/value/with/slashes", safe=""), safe="")
uri = quote(quote("value/with/slashes", safe=""), safe="")
print(self.test_app.get("/server/command/stop/" + uri))
command = self.controller.queue_command.call_args[0][0]
self.assertEqual(Controller.Command.Action.STOP, command.action)
self.assertEqual("/value/with/slashes", command.filename)
self.assertEqual("value/with/slashes", command.filename)

uri = quote(quote(" value with spaces", safe=""), safe="")
print(self.test_app.get("/server/command/stop/" + uri))
Expand Down Expand Up @@ -92,11 +92,11 @@ def side_effect(cmd: Controller.Command):
self.assertEqual(Controller.Command.Action.EXTRACT, command.action)
self.assertEqual("test1", command.filename)

uri = quote(quote("/value/with/slashes", safe=""), safe="")
uri = quote(quote("value/with/slashes", safe=""), safe="")
print(self.test_app.get("/server/command/extract/" + uri))
command = self.controller.queue_command.call_args[0][0]
self.assertEqual(Controller.Command.Action.EXTRACT, command.action)
self.assertEqual("/value/with/slashes", command.filename)
self.assertEqual("value/with/slashes", command.filename)

uri = quote(quote(" value with spaces", safe=""), safe="")
print(self.test_app.get("/server/command/extract/" + uri))
Expand Down Expand Up @@ -128,11 +128,11 @@ def side_effect(cmd: Controller.Command):
self.assertEqual(Controller.Command.Action.DELETE_LOCAL, command.action)
self.assertEqual("test1", command.filename)

uri = quote(quote("/value/with/slashes", safe=""), safe="")
uri = quote(quote("value/with/slashes", safe=""), safe="")
print(self.test_app.get("/server/command/delete_local/" + uri))
command = self.controller.queue_command.call_args[0][0]
self.assertEqual(Controller.Command.Action.DELETE_LOCAL, command.action)
self.assertEqual("/value/with/slashes", command.filename)
self.assertEqual("value/with/slashes", command.filename)

uri = quote(quote(" value with spaces", safe=""), safe="")
print(self.test_app.get("/server/command/delete_local/" + uri))
Expand Down Expand Up @@ -164,11 +164,11 @@ def side_effect(cmd: Controller.Command):
self.assertEqual(Controller.Command.Action.DELETE_REMOTE, command.action)
self.assertEqual("test1", command.filename)

uri = quote(quote("/value/with/slashes", safe=""), safe="")
uri = quote(quote("value/with/slashes", safe=""), safe="")
print(self.test_app.get("/server/command/delete_remote/" + uri))
command = self.controller.queue_command.call_args[0][0]
self.assertEqual(Controller.Command.Action.DELETE_REMOTE, command.action)
self.assertEqual("/value/with/slashes", command.filename)
self.assertEqual("value/with/slashes", command.filename)

uri = quote(quote(" value with spaces", safe=""), safe="")
print(self.test_app.get("/server/command/delete_remote/" + uri))
Expand All @@ -187,3 +187,54 @@ def side_effect(cmd: Controller.Command):
command = self.controller.queue_command.call_args[0][0]
self.assertEqual(Controller.Command.Action.DELETE_REMOTE, command.action)
self.assertEqual('value"with"doublequote', command.filename)


class TestControllerHandlerValidation(BaseTestWebApp):
"""Tests for filename validation in controller handler."""

def test_control_char_newline_rejected(self):
uri = quote(quote("file\nname", safe=""), safe="")
resp = self.test_app.get("/server/command/queue/" + uri, expect_errors=True)
self.assertEqual(400, resp.status_int)
self.controller.queue_command.assert_not_called()

def test_control_char_carriage_return_rejected(self):
uri = quote(quote("file\rname", safe=""), safe="")
resp = self.test_app.get("/server/command/queue/" + uri, expect_errors=True)
self.assertEqual(400, resp.status_int)
self.controller.queue_command.assert_not_called()

def test_control_char_tab_rejected(self):
uri = quote(quote("file\tname", safe=""), safe="")
resp = self.test_app.get("/server/command/queue/" + uri, expect_errors=True)
self.assertEqual(400, resp.status_int)
self.controller.queue_command.assert_not_called()

def test_control_char_soh_rejected(self):
uri = quote(quote("file\x01name", safe=""), safe="")
resp = self.test_app.get("/server/command/queue/" + uri, expect_errors=True)
self.assertEqual(400, resp.status_int)
self.controller.queue_command.assert_not_called()

def test_control_char_del_rejected(self):
uri = quote(quote("file\x7fname", safe=""), safe="")
resp = self.test_app.get("/server/command/queue/" + uri, expect_errors=True)
self.assertEqual(400, resp.status_int)
self.controller.queue_command.assert_not_called()

def test_normal_filename_accepted(self):
def side_effect(cmd: Controller.Command):
cmd.callbacks[0].on_success()

self.controller.queue_command = MagicMock()
self.controller.queue_command.side_effect = side_effect

resp = self.test_app.get("/server/command/queue/normal_file.txt")
self.assertEqual(200, resp.status_int)
command = self.controller.queue_command.call_args[0][0]
self.assertEqual("normal_file.txt", command.filename)

def test_path_traversal_rejected(self):
uri = quote(quote("../etc/passwd", safe=""), safe="")
resp = self.test_app.get("/server/command/queue/" + uri, expect_errors=True)
self.assertEqual(400, resp.status_int)
3 changes: 3 additions & 0 deletions src/python/web/handler/controller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ def _validate_filename(file_name: str) -> bool:
# Reject embedded null bytes (path truncation attack)
if "\x00" in file_name:
return False
# Reject control characters (0x00-0x1F, 0x7F)
if any(ord(c) < 0x20 or ord(c) == 0x7F for c in file_name):
return False
# Reject absolute paths
if os.path.isabs(file_name):
return False
Expand Down
Loading