Skip to content

bug: Block read_pickle and class definitions, restrict custom CSV field to read_csv() only#6257

Draft
chloebyun-wd wants to merge 1 commit intomainfrom
bug/523/remote-code-execution-vulneratbility-in-csv-agent
Draft

bug: Block read_pickle and class definitions, restrict custom CSV field to read_csv() only#6257
chloebyun-wd wants to merge 1 commit intomainfrom
bug/523/remote-code-execution-vulneratbility-in-csv-agent

Conversation

@chloebyun-wd
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances security by introducing a specialized validator for custom CSV reading and expanding the list of forbidden Python patterns to prevent unsafe deserialization and class definitions. A security concern was raised regarding the regex for read_pickle, suggesting a broader match to prevent bypasses via variable assignment.

{ pattern: /\b__module__\b/g, reason: '__module__ (module reflection)' }
{ pattern: /\b__module__\b/g, reason: '__module__ (module reflection)' },
// Unsafe deserialization — read_pickle() executes arbitrary Python objects
{ pattern: /\bread_pickle\s*\(/g, reason: 'read_pickle() (unsafe deserialization / RCE)' },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The regex /�read_pickles*(/g only matches direct function calls. It can be bypassed by assigning the function to a variable. Using /�read_pickle�/g is more secure as it catches any reference to the forbidden function.

Suggested change
{ pattern: /\bread_pickle\s*\(/g, reason: 'read_pickle() (unsafe deserialization / RCE)' },
{ pattern: /\bread_pickle\b/g, reason: 'read_pickle (unsafe deserialization / RCE)' },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant