Fix(systemutils): reject credential theft patterns in execute_script#475
Closed
Jean-Regis-M wants to merge 1 commit intoGenAI-Security-Project:mainfrom
Closed
Fix(systemutils): reject credential theft patterns in execute_script#475Jean-Regis-M wants to merge 1 commit intoGenAI-Security-Project:mainfrom
Jean-Regis-M wants to merge 1 commit intoGenAI-Security-Project:mainfrom
Conversation
Root cause: execute_script lacks content validation, allowing scripts with /etc/passwd references to succeed and be recorded as completed. Solution: Add dangerous patterns check before processing script_content, raising ValueError when credential theft indicators detected. Impact: - Minimal diff (4 lines added) - No breaking changes for valid scripts - Deterministic rejection of credential theft - Matches Bug_180 fix pattern Signed-off-by: JEAN REGIS <240509606@firat.edu.tr>
Collaborator
|
this is intentional and by design. |
Contributor
Author
|
Came to realize it! Thank you sir! |
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.
fixes #402
Problem
execute_scriptinfinbot/mcp/servers/systemutils/server.pyaccepted any script content andreturned a
"completed"success response unconditionally, including scripts containing credentialtheft indicators such as
cat /etc/passwdorcat /etc/shadow. This allowed silent exfiltrationpaths to be recorded as successful executions with zero rejection.
Root Cause
The
dangerous_patternslist (established in the Bug_180 fix) was never applied toexecute_script'sscript_contentparameter. The function went directly from entry →logger.warning→ success
return, with no content scanning at any point in between.Classification: Validation gap missing input sanitization before response generation.
Solution
Insert a
dangerous_patternscheck before thelogger.warningcall. On match, raiseValueErrorimmediately, the success path is never reached.Patch Scope
finbot/mcp/servers/systemutils/server.pyexecute_script(lines 172–190)ValueErroris expected per testEdge Cases
script_content = ""inoperator catches it → rejectsVerification
Minimal reproducible case:
Regression test commands:
Task Checklist
execute_script.cat /etc/passwd,cat /etc/shadow,/etc/passwd)logger.warningandreturnno success path reachable on matchValueErrorraised with descriptive pattern messagetest_su_exec_004)test_su_exec_001)