Skip to content

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

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.

Sign up for GitHub

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

Open
colin-d-fried wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions vulnerable_deserialization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import pickle
import json
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot Mar 26, 2026

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.

import yaml
import marshal
from flask import Flask, request
Expand All @@ -9,7 +9,7 @@
def load_data():
data = request.data

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

@cursor cursor Bot Mar 26, 2026

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


return str(obj)

Expand Down
Loading