Skip to content

Commit

Permalink
Avoid option injection for usernames starting with "-"
Browse files Browse the repository at this point in the history
This is not a security problem as untrusted qubes have no control over
the user used in a qrexec command and any qube that can arbitrarily
modify qrexec policy has much easier ways to get control of dom0.

(cherry picked from commit 82483ef)
  • Loading branch information
DemiMarie authored and marmarek committed May 12, 2023
1 parent ff36122 commit 57ce55f
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
2 changes: 1 addition & 1 deletion qrexec/policy/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ async def execute(self, caller_ident):
if dispvm:
qrexec_opts.append("-W")
try:
command = [QREXEC_CLIENT] + qrexec_opts + [cmd]
command = [QREXEC_CLIENT] + qrexec_opts + ["--", cmd]
process = await asyncio.create_subprocess_exec(*command)
await process.communicate()
finally:
Expand Down
5 changes: 5 additions & 0 deletions qrexec/tests/policy_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -1785,6 +1785,7 @@ async def _test_120_execute(self, mock_subprocess, mock_qubesd_call):
"-c",
"some-ident",
"-E",
"--",
"DEFAULT:QUBESRPC test.Service+argument test-vm1",
),
unittest.mock.call().communicate(),
Expand Down Expand Up @@ -1823,6 +1824,7 @@ async def _test_121_execute_dom0(self, mock_subprocess, mock_qubesd_call):
"-c",
"some-ident",
"-E",
"--",
"QUBESRPC test.Service+argument test-vm1 name dom0",
]
)
Expand Down Expand Up @@ -1863,6 +1865,7 @@ async def _test_121_execute_dom0_keyword(
"-c",
"some-ident",
"-E",
"--",
"QUBESRPC test.Service+argument test-vm1 keyword adminvm",
),
unittest.mock.call().communicate(),
Expand Down Expand Up @@ -1918,6 +1921,7 @@ async def _test_122_execute_dispvm(self, mock_subprocess, mock_qubesd_call):
"some-ident",
"-E",
"-W",
"--",
"DEFAULT:QUBESRPC test.Service+argument test-vm1",
),
unittest.mock.call().communicate(),
Expand Down Expand Up @@ -1964,6 +1968,7 @@ async def _test_123_execute_already_running(
"-c",
"some-ident",
"-E",
"--",
"DEFAULT:QUBESRPC test.Service+argument test-vm1",
),
unittest.mock.call().communicate(),
Expand Down

0 comments on commit 57ce55f

Please sign in to comment.