Re: pf: u_int32_t conn counter underflow in pf_src_tree_remove_state()

On 2026/08/06 11:31, Janak Trivedi wrote:
> Dear OpenBSD Team,
> 
> I am writing to report a bug in the PF packet filter that causes the per-source connection
> counter (conn) to underflow, resulting in legitimate connection being rejected with source rule
> tracking applied.
> 
> The bug was discovered during investigation of false positive IP blocking in a production PF
> firewall environment and has been confirmed through controlled lab testing.
> 
> ================================================================
> SYSTEM INFORMATION
> ================================================================
> OpenBSD Version:  7.8
> Architecture:     amd64
> 
> ================================================================
> AFFECTED FILE
> ================================================================
> File:     sys/net/pf.c
> Function: pf_src_tree_remove_state()
> 
> ================================================================
> BUG DESCRIPTION
> ================================================================
> 
> The conn counter (u_int32_t) in struct pf_src_node tracks the number of established TCP
> connections from a source IP address. This counter is used to enforce the max-src-conn rule
> option.
> 
> The counter is:
>   INCREMENTED in: pf_src_connlimit()
>                   when TCP 3-way handshake completes
>                   (st->src.tcp_est flag set to 1)
> 
>   DECREMENTED in: pf_src_tree_remove_state()
>                   when ANY state is removed where tcp_est=1
> 
> The bug is that tcp_est flag is NEVER cleared when a connection transitions from ESTABLISHED to
> FIN_WAIT, TIME_WAIT or CLOSING state. The flag remains set until the state is physically
> removed from the state table.
> 
> At high connection rates, FIN_WAIT states accumulate in the state table. When these states are
> eventually removed, pf_src_tree_remove_state() decrements the conn counter for each removal.
> Since the conn counter was only incremented once per connection (at ESTABLISHED), but is
> decremented once per state removal (including FIN_WAIT), the total decrements exceed the total
> increments, causing unsigned integer underflow.
> 
> ================================================================
> AFFECTED CODE
> ================================================================
> 
> Current code in pf_src_tree_remove_state():
> 
>   pf_src_tree_remove_state(struct pf_state *st)
>   {
>       u_int32_t         timeout;
>       struct pf_sn_item *sni;
> 
>       while ((sni = SLIST_FIRST(&st->src_nodes)) != NULL) {
>           SLIST_REMOVE_HEAD(&st->src_nodes, next);
> 
>           if (st->src.tcp_est)
>               --sni->sn->conn;        /* BUG: no zero check,
>                                          tcp_est never cleared
>                                          on FIN_WAIT transition */

Surely the bug is that tcp_est isn't cleared, wouldn't this zero
check just be hiding the problem?


> 
>           if (--sni->sn->states == 0) {
>               timeout = st->rule.ptr->timeout[PFTM_SRC_NODE];
>               if (!timeout)
>                   timeout =
>                       pf_default_rule.timeout[PFTM_SRC_NODE];
>               sni->sn->expire = getuptime() + timeout;
>           }
>           pool_put(&pf_sn_item_pl, sni);
>       }
>   }
> 
> Note: The states counter (--sni->sn->states) also has no explicit zero check, however it is
> less susceptible to underflow because states is incremented for every new state creation 
> (including SYN_SENT) whereas conn is only incremented at ESTABLISHED, meaning states >= conn in
> normal operation.
> 
> ================================================================
> IMPACT
> ================================================================
> 
> When underflow occurs:
> 
>   conn counter value:      ~4,294,967,105  (near 2^32)
>   max-src-conn threshold:       200,000    (typical value)
> 
>   4,294,967,105 >> 200,000
> 
> Every subsequent connection from the affected source IP immediately exceeds max-src-conn
> threshold, causing:
> 
>   1. Source IP added to overload table (<aggressive>)
>   2. ALL connections from that IP blocked
>   3. IP remains blocked until src.track node expires
>      (typically 10+ seconds after last state removed)
>   4. If administrator manually removes IP from overload table without killing states and
> waiting for src.track expiry, IP is immediately re-blocked on next connection attempt
> 
> This causes legitimate source IPs to be permanently blocked, appearing as random false
> positives in production environments.
> 
> ================================================================
> REPRODUCTION
> ================================================================
> 
> PF rule to reproduce:
> 
>   table <aggressive> persist
> 
>   pass log quick proto tcp from any to <target> port 80 \
>       flags S/SA keep state ( \
>       max 10000000, \
>       max-src-states 999999, \
>       max-src-conn 999999, \
>       max-src-conn-rate 999999/10, \
>       overload <aggressive>, \
>       tcp.first 5, tcp.opening 5, \
>       tcp.established 3600, tcp.closing 900, \
>       tcp.finwait 30, tcp.closed 30, \
>       adaptive.start 6000000, \
>       adaptive.end 12000000)
> 
> Note: No flush option is used. The bug occurs regardless of flush setting (flush, flush global,
> or no flush).
> Traffic generation (using vegeta):
> 
>   echo "GET http://<target>/" | vegeta attack -rate=1000/s -duration=10s -timeout=5s -header=
> "Connection: close" | vegeta report
> 
> Monitoring command to observe underflow:
> 
>   pfctl -s Sources
> 
> Expected observation:
>   During test:   connections counter rises normally
>   After test:    FIN_WAIT states accumulate
>   During expiry: connections counter underflows to ~2^32
>   Final value:   4,294,967,296 - (peak_FIN - peak_conn)
> 
> ================================================================
> PROPOSED FIX
> ================================================================
> 
> Option 1 — Minimal fix (add zero check):
> 
>   /* CURRENT (buggy): */
>   if (st->src.tcp_est)
>       --sni->sn->conn;
> 
>   /* FIXED (safe): */
>   if (st->src.tcp_est && sni->sn->conn > 0)
>       --sni->sn->conn;
> 
>   Pros: Simple one-line change
>         Prevents underflow completely
>         Minimal risk of regression
>   Cons: conn may slightly overcount during high-rate
>         FIN_WAIT accumulation (benign)
> 
> 
> ================================================================
> TEST ENVIRONMENT
> ================================================================
> 
> Firewall:  OpenBSD
> Client:    OpenBSD with vegeta package
> Server:    OpenBSD with nginx package
> Traffic:   vegeta 12.12.0
> 
> ================================================================
> 
> Thank you for your time and for maintaining OpenBSD and PF. I am happy to provide additional
> information, test results, or packet captures if helpful.
> 
> Regards,
> Janak Trivedi
> Stockholm Sweden