Skip to content

[Security] Fix CodeQL alert #34: Deserialization of user-controlled data#86

Open
colin-d-fried wants to merge 1 commit into
mainfrom
security/codeql-34-deser-load-fix
Open

[Security] Fix CodeQL alert #34: Deserialization of user-controlled data#86
colin-d-fried wants to merge 1 commit into
mainfrom
security/codeql-34-deser-load-fix

Conversation

@colin-d-fried
Copy link
Copy Markdown
Owner

@colin-d-fried colin-d-fried commented Mar 26, 2026

Summary

Fixes CodeQL alert #34: Deserialization of user-controlled data

Field Value
Severity critical
File vulnerable_deserialization.py
CWE CWE-502
Alert CodeQL Alert #34

Fix Applied

See the diff for the specific secure coding change applied.

Fixes #36


Note

High Risk
Changes request deserialization behavior and removes the pickle import while other code paths still call pickle.loads, which can cause runtime failures and leaves other unsafe deserializations unchanged.

Overview
Updates vulnerable_deserialization.py to stop using pickle.loads on raw POST body data in the /load route, switching to json.loads and replacing the pickle import with json.

Other routes and helpers still call pickle.loads (now without importing pickle), so the PR likely introduces NameError at runtime and does not address remaining user-controlled deserialization sites.

Written by Cursor Bugbot for commit 6ecbdfc. This will update automatically on new commits. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

data = request.data

obj = pickle.loads(data)
obj = json.loads(data)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Removed pickle import but kept pickle usage

High Severity

The import pickle statement was removed and replaced with import json, but pickle.loads() is still called on line 20 in restore_session(). This will cause a NameError at runtime when the /session endpoint is hit. Additionally, the security fix is incomplete — this endpoint still deserializes user-controlled data with pickle, which was the exact vulnerability the PR aims to fix.

Fix in Cursor Fix in Web

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

@@ -1,4 +1,4 @@
import pickle
import json
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Removed import pickle but pickle is still used in multiple places, causing NameError at runtime

The PR replaces import pickle with import json but only migrates one of the six pickle.loads() call sites to json.loads(). The remaining five usages at lines 20, 29, 40, 50, and 53 will raise NameError: name 'pickle' is not defined at runtime. This breaks the /session endpoint, /import endpoint, deserialize_object(), and both DataProcessor methods.

Prompt for agents
In vulnerable_deserialization.py, the `import pickle` was removed on line 1 and replaced with `import json`, but pickle is still referenced on lines 20, 29, 40, 50, and 53. Either:
1. Add `import pickle` back (line 1) alongside `import json` to restore the remaining pickle usages, OR
2. Migrate ALL pickle.loads() call sites to use json.loads() (or another safe deserialization method) to complete the intended security fix.

Option 2 is strongly recommended since pickle.loads on untrusted input is a remote code execution vulnerability. Replace pickle.loads() with json.loads() at lines 20, 29, 40, 50, and 53, adjusting for the fact that json.loads expects str/bytes of JSON, not pickle byte streams.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

[CodeQL #34] Deserialization of user-controlled data

1 participant