Skip to content

Commit 213cbf6

Browse files
authored
validate variable names in copyToGlobals debug handler (#1537)
**Expression injection via unvalidated variable names in copyToGlobals** While reading through the debug handlers I noticed `copyToGlobals` drops the client-supplied `dstVariableName` straight into `globals()['{dst_var_name}']` and forwards that to `setExpression`, and hands `srcVariableName` over as the value expression, with neither of them checked. Its sibling `richInspectVariables` already runs `str.isidentifier` over the name it evaluates, so the omission here stood out. A name such as `x'] or __import__('os').system('id') or globals()['y` closes the quoted string and injects an arbitrary expression that debugpy then evaluates in the debuggee frame; the forwarded `setExpression` payload showed the escaped quotes plainly once I logged it. Left as is this lets a malformed or hostile debug request steer evaluation well beyond a variable copy. The change rejects any `dstVariableName`/`srcVariableName` that is not a valid identifier before the expression is built, mirroring the guard the sibling handler already applies. Added a regression under the existing debugger tests that confirms such a request is refused locally rather than forwarded.
2 parents 484493c + 5e2f6db commit 213cbf6

2 files changed

Lines changed: 30 additions & 0 deletions

File tree

‎ipykernel/debugger.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,15 @@ async def copyToGlobals(self, message):
681681
src_var_name = message["arguments"]["srcVariableName"]
682682
src_frame_id = message["arguments"]["srcFrameId"]
683683

684+
if not str.isidentifier(dst_var_name) or not str.isidentifier(src_var_name):
685+
return {
686+
"type": "response",
687+
"request_seq": message["seq"],
688+
"success": False,
689+
"command": message["command"],
690+
"message": "dstVariableName and srcVariableName must be valid identifiers",
691+
}
692+
684693
expression = f"globals()['{dst_var_name}']"
685694
seq = message["seq"]
686695
return await self._forward_message(

‎tests/test_debugger.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,27 @@ def my_test():
473473
assert global_var["value"] == local_var["value"] and global_var["type"] == local_var["type"] # noqa: PT018
474474

475475

476+
def test_copy_to_globals_rejects_non_identifier(kernel_with_debug):
477+
# A dstVariableName that is not a valid identifier would break out of the
478+
# single-quoted globals()['...'] expression forwarded to setExpression.
479+
reply = wait_for_debug_request(
480+
kernel_with_debug,
481+
"copyToGlobals",
482+
{
483+
"srcVariableName": "src",
484+
"dstVariableName": "x'] or __import__('os').system('echo pwned') or globals()['y",
485+
"srcFrameId": 0,
486+
},
487+
)
488+
# The request is rejected locally instead of being forwarded to
489+
# setExpression, so the response still carries the copyToGlobals command.
490+
if debugpy:
491+
assert reply["success"] is False
492+
assert reply["command"] == "copyToGlobals"
493+
else:
494+
assert reply == {}
495+
496+
476497
def test_debug_requests_sequential(kernel_with_debug):
477498
# Issue https://github.com/ipython/ipykernel/issues/1412
478499
# Control channel requests should be executed sequentially not concurrently.

0 commit comments

Comments
 (0)