Skip to Content
Course content

391: Avoiding Common Security Pitfalls in Python Code

Click on the "Edit" button in the top corner of the screen to edit your slide content.

I've spent a lot of time reviewing code for junior and mid-level devs, and there is one misconception that keeps popping up, even among people who are great at logic: the belief that f-strings are a "safe" way to build queries because they are a modern, built-in Python feature. I see it all the time—someone writes a database query or a system command using an f-string and thinks, "I'm just using Python's standard way of formatting strings, so it's fine."

It is absolutely not fine. Let me show you why.

The Myth: f-strings protect you from injection

Imagine you're building a simple user lookup tool. You might be tempted to write something like this:

import sqlite3

def get_user_data(username):
    conn = sqlite3.connect("users.db")
    cursor = conn.cursor()
    # This looks clean, right?
    query = f"SELECT * FROM users WHERE username = '{username}'"
    return cursor.execute(query).fetchone()

On the surface, it's readable. But if a user provides the input ' OR '1'='1, your query becomes SELECT * FROM users WHERE username = '' OR '1'='1'. Since '1'='1' is always true, the database will just hand over the first user in the table—likely the admin. You've just opened a massive SQL injection hole because the f-string doesn't know it's building a query; it just knows it's gluing strings together.

The Reality: Let the Driver Handle the Data

The correct approach is to use parameterized queries. You don't build the string yourself; you send the query template and the data as separate arguments to the database driver. The driver then ensures the data is treated as a literal value, not as executable code.

# The secure way
def get_user_data(username):
    conn = sqlite3.connect("users.db")
    cursor = conn.cursor()
    # Use a placeholder (?) and pass the data as a tuple
    query = "SELECT * FROM users WHERE username = ?"
    return cursor.execute(query, (username,)).fetchone()

Now, if a user enters ' OR '1'='1, the database looks for a user whose actual name is literally the string ' OR '1'='1. It fails safely.

Stopping the Shell from Executing Your Input

This same logic applies to the command line. I often see people using os.system() or subprocess.Popen(..., shell=True) to run system tools. Using shell=True is essentially inviting a stranger to run any command they want on your server.

If you have a script that pings a host provided by a user:

import os
host = "google.com"
os.system(f"ping -c 1 {host}")

A malicious user could provide "google.com; rm -rf /" as the host. The shell will execute the ping, and then it will happily try to delete your entire filesystem. To fix this, always pass your commands as a list and set shell=False (which is the default for subprocess.run). This bypasses the shell entirely and executes the binary directly.

import subprocess
# This is safe because the list elements are treated as arguments, not shell commands
subprocess.run(["ping", "-c", "1", host])

The Danger of "Trusting" Pickled Data

Finally, let's talk about pickle. I know it's convenient for saving complex Python objects to a file, but pickle is fundamentally insecure. It doesn't just store data; it stores the instructions on how to reconstruct an object. A crafted pickle file can tell Python to execute os.system('rm -rf /') the moment pickle.load() is called.

My rule of thumb: Never, ever pickle.load() data that came from a network request, a user upload, or any source you don't 100% control. If you need to store data, use json. It's a bit more restrictive—you can't save custom class instances as easily—but it's just data. It can't execute code on your machine.

  • SQL: Use parameterized queries, never f-strings or .format().
  • Shell: Use subprocess.run([]) with a list; avoid shell=True.
  • Serialization: Use json for untrusted data; treat pickle as a potential remote code execution vulnerability.



📋 Practical Task

Hardening a Vulnerable User-Profile API

You have been handed a legacy snippet of a User Profile API that is riddled with security holes. Your task is to rewrite the fetch_profile and check_server_status functions to be secure.

import sqlite3
import os
import subprocess

def fetch_profile(user_id):
    # VULNERABLE: Fix this SQL injection
    conn = sqlite3.connect("app.db")
    cursor = conn.cursor()
    query = f"SELECT bio FROM profiles WHERE id = {user_id}"
    return cursor.execute(query).fetchone()

def check_server_status(ip_address):
    # VULNERABLE: Fix this shell injection
    os.system(f"ping -c 1 {ip_address}")
    return "Ping attempted"

# --- TEST CASES ---
# These should NOT compromise the system when fixed:
print(fetch_profile("1 OR 1=1")) 
print(check_server_status("8.8.8.8; ls -la"))

Requirements:

  • Modify fetch_profile to use parameterized queries so that "1 OR 1=1" is treated as a literal ID.
  • Modify check_server_status to use subprocess.run with a list of arguments and shell=False to prevent command chaining.
  • Ensure the code still performs the intended functionality (querying the DB and pinging the IP).
Rating
0 0

There are no comments for now.

to be the first to leave a comment.