-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Python: Add unsafe deserialization sinks (CWE-502) #13781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Python: Add unsafe deserialization sinks (CWE-502) #13781
Conversation
python/ql/lib/change-notes/2023-07-20-add-unsafe-deserialization-sinks.md
Show resolved
Hide resolved
python/ql/lib/change-notes/2023-07-20-add-unsafe-deserialization-sinks.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Jorge <[email protected]>
Co-authored-by: Jorge <[email protected]>
Co-authored-by: Jorge <[email protected]>
Co-authored-by: Jorge <[email protected]>
Co-authored-by: Jorge <[email protected]>
Co-authored-by: Jorge <[email protected]>
To use positional argument for allow_pickle
RasmusWL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this high quality submission @maikypedia 👍
I've made a few minor changes, but overall everything looked good 🙌
(I'll just take a look at new results before merging 👍)
|
I looked over some of the new sinks we get from this (results) -- we get quite a few cases where it's clearly a path being passed to one of the new sinks, such as in the example below. np.load(os.path.join(graph_work_path, "train_data.npy"), allow_pickle=True)In those cases, it becomes tricky. If user can control the data being loaded, it could still lead to arbitrary code execution through deserialization. It's a little unclear whether to treat this under the path-injection query or not, since the normal rules for a safe path doesn't necessarily apply here (path restricted to specific folder) 🤔 let me be clear: It's not something I would expect you to have solved @maikypedia, just an interesting observation we (CodeQL team) can consider how to approach. |
This pull request adds sinks for unsafe deserialization covering
pandas,numpyandjoblib.