🛡️ Sentinel: [MEDIUM] Add missing deserialization sinks to _SERIALISATION_SINKS#102
🛡️ Sentinel: [MEDIUM] Add missing deserialization sinks to _SERIALISATION_SINKS#102tachyon-beep wants to merge 1 commit into
Conversation
Co-authored-by: tachyon-beep <544926+tachyon-beep@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR updates Wardline’s level-2 taint propagation so that additional deserialization APIs are treated as representation-boundary sinks whose results should shed validation provenance (i.e., become UNKNOWN_RAW). This aligns the core propagation behavior with the PY-WL-106 rule’s sink specs and reduces false negatives where deserialized outputs could be incorrectly considered safe downstream.
Changes:
- Added several missing deserialization function FQNs to
_SERIALISATION_SINKSso their return values becomeUNKNOWN_RAW. - Recorded the security learning/update in
.jules/sentinel.mdfor traceability.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/wardline/scanner/taint/variable_level.py | Extends _SERIALISATION_SINKS with additional deserialization sinks to ensure provenance is shed on return values. |
| .jules/sentinel.md | Documents the incident/learning entry related to the added sinks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "dill.load", | ||
| "dill.loads", |
| **Vulnerability:** The static analyzer was missing several unsafe deserialization functions (`dill.load`, `jsonpickle.decode`, `torch.load`, `shelve.open`, `joblib.load`, `pickle.Unpickler.load`) in its `_SERIALISATION_SINKS` mapping. | ||
| **Learning:** Functions that are conditionally safe like `numpy.load` (safe without `allow_pickle=True`) shouldn't be added to universal static block lists without context if it would create false positives, but functions that represent deserialization flows still need inclusion to shed provenance. |
🚨 Severity: MEDIUM
💡 Vulnerability: Several deserialization sinks (like
dill.load,shelve.open,torch.load, etc.) recently added to thePY-WL-106rule specifications in_SINK_SPECSwere missing from the core taint propagation logic_SERIALISATION_SINKS. This causes the analyzer to incorrectly retain validation provenance for these functions, leading to false negatives (the analyzer doesn't know their output should becomeUNKNOWN_RAW).🎯 Impact: If
dill.loador other similar sinks returned data that was later used in another dangerous sink, the analyzer might consider it safe, creating blind spots for potential downstream injections or RCEs.🔧 Fix: Added
dill.load,dill.loads,jsonpickle.decode,joblib.load,torch.load,shelve.open, andpickle.Unpickler.loadto_SERIALISATION_SINKS. We explicitly excludednumpy.loadfrom this list because it is safe by default (requiresallow_pickle=Trueto unpickle objects) and adding it unconditionally to_SERIALISATION_SINKScreates false positives (e.g. failing thePY-WL-101clean test cases).✅ Verification: Verified by ensuring the
make test,make lint, andmake typecheckall complete successfully with the missing sinks correctly shedding provenance.PR created automatically by Jules for task 7465837750491915562 started by @tachyon-beep