fix(transform_editor): escape output name to prevent code injection#182
Open
pabloinigoblasco wants to merge 2 commits into
Open
fix(transform_editor): escape output name to prevent code injection#182pabloinigoblasco wants to merge 2 commits into
pabloinigoblasco wants to merge 2 commits into
Conversation
buildTransformScript concatenated the user-controlled id/name (the
output-name
field in Single mode, the suffix in Batch mode) unescaped into a
double-quoted
string literal of the generated Lua/Python script, which is then
compiled and
run. A name containing a quote closes the literal early and the rest
is parsed
as code: e.g. an output name of `a"; import os; os.system("...") ;
z="` executes
arbitrary code in the host process when the transform is created
(Python
backend; the Luau sandbox blocks os/io so its impact is bounded).
Add escapeForStringLiteral() and escape id/name (quote, backslash,
newlines)
before embedding them. buildTransformScript is the single
script-generation
point, so this covers both call sites (Single and Batch) and both
backends. No
behaviour change for normal names.
…itor-code-injection
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
buildTransformScriptconcatenated the user-controlled id/name (the output-name field in Single mode, the suffix in Batch mode) unescaped into a double-quoted string literal of the generated Lua/Python script, which is then compiled and run. A name containing a quote closes the literal early and the rest is parsed as code: e.g. an output name ofa"; import os; os.system("..."); z="executes arbitrary code in the host process when the transform is created (Python backend; the Luau sandbox blocksos/ioso its impact is bounded).Add
escapeForStringLiteral()and escape id/name (quote, backslash, newlines) before embedding them.buildTransformScriptis the single script-generation point, so this covers both call sites (Single and Batch) and both backends. No behaviour change for normal names.How it was tested
Reproduced with the Python backend: Transform Editor → Single, one input series, function
return value, and the output name:Ran the Python script exactly as generated by the plugin:
os.system(...)runs →/tmp/pj_pwnedis created (arbitrary code executed).name = "a\"; import os; os.system(...)"→ it stays an inert string →/tmp/pj_pwnedis not created.The transform is created in both cases; the difference is that with the fix the name is never executed.
Contents
toolbox_transform_editor/transform_editor_plugin.cpp:escapeForStringLiteral()helper + escape id/name inbuildTransformScriptfor both Single and Batch modes.