Safeguard
Application Security

The Python code review security checklist: eval, pickle, and shell=True

Bandit ships named checks for eval, pickle, and shell=True — B307, B301, B602 — yet these three smells still slip past manual review into production Python.

Safeguard Research Team
Research
6 min read

Python's standard library makes several dangerous operations look completely ordinary, and that is exactly why they keep showing up in production code. pickle.loads() reads like any other deserializer, but Python's own documentation states plainly that "the pickle module is not secure — only unpickle data you trust," because a crafted pickle stream can execute arbitrary code on load. That exact weakness class, CWE-502, was still being found in actively maintained libraries as recently as 2025: CVE-2025-61765 documented a remote code execution path in python-socketio versions before 5.14.0, triggered by unsafe pickle deserialization in multi-server deployments (tracked as GHSA-g8c6-8fjj-2r4m). Meanwhile subprocess.run(cmd, shell=True) and eval(user_input) are one keystroke away from their safe equivalents, which is precisely why they survive code review — nothing about the syntax signals danger. Bandit, the PyCQA project's official Python security linter, has carried dedicated rule IDs for these exact patterns for years: B301 for unsafe deserialization, B602 for subprocess_popen_with_shell_equals_true, B307 for eval. This post is a working checklist — the smells worth flagging by eye, the CWE they map to, and the automated rule that should catch them if a human misses one.

Why is pickle.loads() a code smell wherever untrusted data can reach it?

pickle.loads() is dangerous because Python's pickle protocol can serialize not just data but instructions — specifically, a call to __reduce__ that pickle will execute on load to reconstruct an object. An attacker who controls the pickled bytes controls that call, which means deserializing an untrusted pickle is equivalent to running attacker-supplied code. This maps to CWE-502 (Deserialization of Untrusted Data) and Bandit rule B301. It is not a theoretical risk: CVE-2025-61765 in python-socketio showed the pattern playing out in a real, actively-used library — pickle deserialization in the multi-server message-passing path allowed RCE before the 5.14.0 fix. During review, treat every pickle.load, pickle.loads, and cPickle equivalent as a hard stop unless the input source is fully trusted and never crosses a network or queue boundary. Safer alternatives — JSON, or hmac-signed pickle payloads if the format is truly required — should replace it wherever the data ever originated outside the process.

What makes subprocess shell=True different from a safe subprocess call?

subprocess.run(), subprocess.Popen(), and os.system() all accept a command, but shell=True changes how that command is interpreted: instead of executing a program with a fixed argument list, it hands the whole string to /bin/sh (or cmd.exe on Windows) for shell parsing — meaning ;, |, backticks, and $( ) are all live metacharacters. If any part of that string is built from user input, attacker-controlled text can chain additional commands. This is CWE-78 (OS Command Injection), and Bandit flags it specifically as B602 ( subprocess_popen_with_shell_equals_true ), distinct from the general B603/B607 checks on subprocess calls without a shell. The fix during review is mechanical: pass a list of arguments (["ls", "-l", user_path]) with shell=False (the default) instead of an interpolated string, so the OS executes the program directly with no shell parsing step at all.

Why do eval() and exec() deserve the same scrutiny as SQL string concatenation?

eval() and exec() compile and run arbitrary Python source at runtime, which means any attacker-influenced string passed to either one is effectively a code-injection sink — the same shape of bug as building a SQL query with string concatenation, just with the Python interpreter as the target instead of a database engine. This is CWE-95 (Eval Injection), covered by Bandit as B307. It shows up in surprisingly mundane places: a "calculator" endpoint that evals a math expression, or a config loader that evals a string to build a dict. The equivalent SQL smell — "SELECT * FROM users WHERE id = " + user_id — is CWE-89 and Bandit B608, and reviewers should scan for both with the same reflex: any function that turns a string into executable logic (SQL, shell, or Python bytecode) is a sink, and the only question worth asking is whether untrusted input can reach it. ast.literal_eval() is the safe substitute when the goal is only to parse a Python literal, not run code.

Does yaml.load() carry the same risk as pickle, and how do you tell?

Yes — PyYAML's yaml.load() without an explicit Loader=yaml.SafeLoader argument can instantiate arbitrary Python objects from the YAML document, which is functionally the same deserialization risk as pickle, just reached through a different file format. This was significant enough that PyYAML shipped CVE-2020-14343 to track a bypass of an earlier partial fix (CVE-2017-18342), and the library changed yaml.load()'s default loader in version 5.1 from the fully unsafe Loader to the comparatively safer FullLoader, pairing the change with a deprecation warning urging callers to pick a loader explicitly — a nudge that became a hard requirement (no implicit default at all) in PyYAML 6.0. During review, flag any yaml.load(data) call missing an explicit SafeLoader (or yaml.safe_load(data), the recommended shorthand) the same way you'd flag a bare pickle.loads(). Config files that look static today can become attacker-influenced tomorrow if they're ever loaded from a webhook payload, an uploaded file, or a shared cache — treat the loader choice as a permanent property of the code, not a one-time judgment call about today's data source.

What other Bandit-named smells belong on the same checklist?

A handful of other Bandit rules round out a practical Python review checklist because they map to real, recurring weakness classes rather than style preferences. B105/B106 flag hardcoded password or secret strings (CWE-798) sitting directly in source. B324 flags use of broken or weak hashes like MD5 or SHA1 for security purposes (CWE-327) — still common for "quick" integrity checks that later get relied on for something sensitive. B103/B108 flag overly permissive file permissions and predictable temp-file paths, both of which have enabled real privilege-escalation and race-condition bugs. None of these require exotic input to exploit; they're smells you can catch by grepping for the pattern, which is exactly why a linter should own the first pass and a human reviewer should own the judgment call: is this string actually a secret, is this hash actually used for security, is this shell call actually reachable by attacker-controlled data.

How Safeguard Helps

A checklist only works if it's applied on every diff, not just the ones a reviewer happens to read carefully — which is what automated source-to-sink analysis is for. Safeguard's first-party SAST engine supports Python as a phase-1 language and traces untrusted input from a source — a request parameter, a CLI argument, a file read — through the call graph to a dangerous sink like subprocess, eval, or pickle.loads, returning a CWE-mapped finding with the full dataflow trace rather than a bare line number. Run it locally or in CI with safeguard appsec sast --dir ./src --tenant-id <tenant> --product-id <product> , and results land in the same findings view as your SCA, secrets, and container scans, so a reviewer checking off "did anyone flag the shell=True in this PR" gets an answer automatically instead of relying on memory.

Never miss an update

Weekly insights on software supply chain security, delivered to your inbox.