SNOW-3417310 fix SQL injection in snow cortex complete/translate#2993
Draft
sfc-gh-olorek wants to merge 1 commit into
Draft
SNOW-3417310 fix SQL injection in snow cortex complete/translate#2993sfc-gh-olorek wants to merge 1 commit into
sfc-gh-olorek wants to merge 1 commit into
Conversation
The --model argument of `snow cortex complete --backend sql` and the --from / --to arguments of `snow cortex translate` were interpolated verbatim into the generated SELECT SNOWFLAKE.CORTEX.* SQL. A single quote in the value closed the string literal, and because the query is sent through execute_stream the trailing characters were parsed as additional statements, letting a caller smuggle arbitrary SQL into the session (e.g. --model "evil'; SELECT 1; --"). Apply the existing _escape_input helper to these three arguments as well so any single quote or backslash is escaped before it hits the SQL text. Added unit tests covering each parameter.
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.
Pre-review checklist
Changes description
CortexManagerbuilds its SQL with Python f-strings and sends it throughexecute_stream, which tokenizes the text on;. Three arguments were interpolated without any escaping:snow cortex complete --backend sql --model <value>(both the text-prompt andPARSE_JSONbranches inCortexManager.complete)snow cortex translate --from <value>snow cortex translate --to <value>Because a single quote inside any of those values closes the surrounding SQL string literal, a caller could smuggle arbitrary SQL into the session. Example:
would emit
SELECT SNOWFLAKE.CORTEX.COMPLETE('evil'; SELECT 1; --', 'prompt') AS CORTEX_RESULT;and the injectedSELECT 1would execute under the user's session.The text/source-document/question arguments on the same call sites already went through
CortexManager._escape_input. This change routes the model and language arguments through the same helper (withstr(...)to cover theNewTypewrappers), so single quotes and backslashes are escaped consistently across all five interpolated values.Tests
Added three unit tests in
tests/cortex/test_cortex_commands.pythat pass a'; SELECT 1; --payload through--model,--to, and--fromrespectively and assert the emitted SQL escapes the quote. Fulltests/cortex/test_cortex_commands.pysuite (21 tests) passes locally.Jira: SNOW-3417310