feat: rate-limit-accept-inverted smell — the fourth quadrant admits only the excess (v1.26.0)

README.md

@@ -12,7 +12,7 @@
 
 ![FirewallScope on the port-forward sample: the input pane shows an iptables-save dump with a filter table (INPUT, FORWARD, OUTPUT) and a nat table publishing a web app (DNAT to 10.0.0.10:80/:443) and an SSH jumpbox (DNAT 2222 to 10.0.0.20:22) plus an outbound MASQUERADE. The graph view on the right renders both tables as compound boxes with chains colour-coded by policy (INPUT / FORWARD red for DROP, OUTPUT and nat chains green for ACCEPT).](screenshots/screenshot.png)
 
-🟢 **v1.0.0** — released. Four parsers (`iptables-save`, `ip6tables-save`, `nft list ruleset`, `ufw status verbose`) with auto-detect. Structural graph with chain tooltips (action distribution + comments) and PNG / SVG export. **Diff** view (added / removed / reordered / policy-change). **Linter** with 34 smells (permissive-accept, exposed-admin-port, wide-open-port-range, overbroad-source-trust, missing-input-drop, shadowed-rule, fallthrough-accept, rule-after-policy-drop, loopback-not-allowed, exposed-via-dnat, forward-no-default-deny, missing-established-accept, masquerade-any-source, drop-without-log, missing-invalid-drop, icmpv6-blocked, icmp-pmtud-blocked, unused-chain, unlimited-log, duplicate-rule, unlimited-icmp-echo, unrestricted-egress, mac-based-trust, admin-port-no-rate-limit, log-tcp-sequence, missing-loopback-spoof-drop, ipv6-unfiltered, dnat-forward-blocked, bogon-source-accept, log-without-prefix, dnat-unscoped, dnat-no-hairpin, rate-limit-not-per-source, rate-limit-drop-inverted) and JSON / Markdown export. **Trace a packet** across the full pipeline: `INPUT` / `FORWARD` / `OUTPUT` directions, interface matches (`-i` / `-o` / `iifname` / `oifname`) modeled, and the complete **NAT model** — `nat/PREROUTING` and `nat/OUTPUT` DNAT / REDIRECT rewrite the packet *before* filter, `nat/POSTROUTING` SNAT / MASQUERADE runs *after* filter accepts. The verdict line surfaces every rewrite, the trace log shows step-by-step matches, and the Graph view highlights visited chains live.
+🟢 **v1.0.0** — released. Four parsers (`iptables-save`, `ip6tables-save`, `nft list ruleset`, `ufw status verbose`) with auto-detect. Structural graph with chain tooltips (action distribution + comments) and PNG / SVG export. **Diff** view (added / removed / reordered / policy-change). **Linter** with 35 smells (permissive-accept, exposed-admin-port, wide-open-port-range, overbroad-source-trust, missing-input-drop, shadowed-rule, fallthrough-accept, rule-after-policy-drop, loopback-not-allowed, exposed-via-dnat, forward-no-default-deny, missing-established-accept, masquerade-any-source, drop-without-log, missing-invalid-drop, icmpv6-blocked, icmp-pmtud-blocked, unused-chain, unlimited-log, duplicate-rule, unlimited-icmp-echo, unrestricted-egress, mac-based-trust, admin-port-no-rate-limit, log-tcp-sequence, missing-loopback-spoof-drop, ipv6-unfiltered, dnat-forward-blocked, bogon-source-accept, log-without-prefix, dnat-unscoped, dnat-no-hairpin, rate-limit-not-per-source, rate-limit-drop-inverted, rate-limit-accept-inverted) and JSON / Markdown export. **Trace a packet** across the full pipeline: `INPUT` / `FORWARD` / `OUTPUT` directions, interface matches (`-i` / `-o` / `iifname` / `oifname`) modeled, and the complete **NAT model** — `nat/PREROUTING` and `nat/OUTPUT` DNAT / REDIRECT rewrite the packet *before* filter, `nat/POSTROUTING` SNAT / MASQUERADE runs *after* filter accepts. The verdict line surfaces every rewrite, the trace log shows step-by-step matches, and the Graph view highlights visited chains live.
 
 ---
 
@@ -146,6 +146,7 @@ FirewallScope's direction: cover the common firewall surfaces and gradually add
 - [x] **v1.23.0** — New linter smell **`rate-limit-not-per-source`** (warning): the follow-up question to `admin-port-no-rate-limit` — that one asks whether a throttle *exists*, this one whether it is *keyed right*. Plain `-m limit`, `-m hashlimit` without a `srcip` mode, and a bare nft `limit rate` all keep **one token bucket that every client drains together**: an attacker holding the bucket empty with a trickle of SYNs makes the rule drop everyone *else's* connections too, so the "brute-force throttle" doubles as a **remote off-switch for the service** — the classic flaw of the tutorial SYN-flood recipe. The fix is a per-client bucket: `-m hashlimit --hashlimit-mode srcip`, `-m recent`, `-m connlimit` (nft: a `meter` / dynamic set keyed on `ip saddr`, or `ct count`). Scoped to **TCP accepts on purpose**: global caps are the right tool where *total* volume is the concern — ICMP echo (what `unlimited-icmp-echo` prescribes) and UDP amplification ceilings stay unflagged. Mutually exclusive with `admin-port-no-rate-limit` by construction (no throttle → that one; shared throttle → this one; pinned by a test). The sloppy router's second 8443 accept "throttles brute force" with plain `-m limit` and trips it. Linter goes from 31 to 32 smells.
 - [x] **v1.24.0** — New linter smell **`rate-limit-drop-inverted`** (warning): the DROP side of the rate-limit story — the bucket points the **wrong way**. `-m limit` (always), `-m hashlimit` without `--hashlimit-above`, and nft `limit rate` without `over` all match traffic while it is *under* the rate: the right direction for an ACCEPT ("let this much through"), exactly backwards on a DROP — the rule discards the first N packets of every interval (calm, legitimate traffic) and once the bucket runs dry **the flood sails past it** to the rules below. The tutorial SYN-flood recipe with `-j DROP` on the limit line degrades the service on a quiet day and protects nothing under attack. Judged for **every protocol** (dropping calm pings is as backwards as dropping calm SYNs) and per-source keying does not save it (a `meter { ip saddr limit rate 3/minute } drop` just inverts per client). **REJECT is exempt on purpose**: an under-limit REJECT is the classic reflector-avoidance recipe — cap how many polite rejections leave per second, let the excess fall through to a silent drop. Alongside it, **`rate-limit-not-per-source` now judges drop-the-excess rules too**: `limit rate over` / `--hashlimit-above` with one global bucket lets an attacker push everyone into the excess, so the flood protection drops legitimate packets alongside the flood — same off-switch, DROP form (still TCP-only; mutually exclusive with the inverted smell by construction: under-limit direction lands there, over-limit sharing lands here). The sloppy router gains the tutorial SYN-flood pair on port 80 and trips it. Linter goes from 32 to 33 smells.
 - [x] **v1.25.0** — New linter smell **`icmp-pmtud-blocked`** (warning): the IPv4 sibling of `icmpv6-blocked`, scoped to the one message IPv4 genuinely cannot live without — ICMP **type 3** (destination-unreachable, whose code 4 is *fragmentation-needed*). TCP sends every segment with DF set and relies on routers answering "too big" with that message; a firewall that swallows it turns any smaller-MTU path (VPN, PPPoE, tunnels) into a **black hole**: the handshake's small packets pass, the payload's full-size ones vanish, connections just hang. Two triggers, one id: **rule-level** — the classic `-p icmp -j DROP` "block ping" rule with no covering accept above it (the order matters: the same drop *below* the conntrack accept is fine); **chain-level** — a deny-posture INPUT / FORWARD that accepts real traffic but never ICMP. Quiet when ICMP is accepted broadly or type 3 explicitly, or when a **RELATED**-state accept lets conntrack pass the errors for tracked connections — ESTABLISHED alone is not enough, ICMP errors about a connection are RELATED. Also quiet for an **empty deny-posture FORWARD** (a non-forwarding host's correct config passes no traffic whose PMTUD could break), for ufw (before.rules accepts dest-unreach invisibly) and ip6tables (the twin's job). New **iptables (ICMP blocked — no PMTUD)** sample trips both variants. Linter goes from 33 to 34 smells.
+- [x] **v1.26.0** — New linter smell **`rate-limit-accept-inverted`** (warning): the **fourth quadrant** of the rate-limit matrix, and the only one still unjudged. Under-limit ACCEPT = a correct throttle (judged only for bucket sharing); under-limit DROP = `rate-limit-drop-inverted`; over-limit DROP = the correct drop-the-excess recipe (judged for sharing); **over-limit ACCEPT = this**: `-m hashlimit --hashlimit-above … -j ACCEPT` / nft `limit rate over … accept` admits **only the traffic above the rate** — calm, legitimate traffic never matches and falls through to the default deny, so the service is dead on a quiet day and springs to life only under flood. Typically a half-fix: someone flips the tutorial recipe's matcher to `over` but forgets to flip the verdict. Like its DROP sibling it judges **every protocol** and **per-source keying does not save it** (a meter `{ ip saddr limit rate over 3/minute } accept` just inverts per client — pinned by test). **Mutually exclusive with `rate-limit-not-per-source` by construction**: that detector now waves over-limit ACCEPTs through (prescribing `srcip` keying would "fix" a rule whose real problem is the verdict). The sloppy router gains an SMTP "guard" pasted backwards — per-source, even — and trips it. Linter goes from 34 to 35 smells.
 
 ## Stack
 

index.html

@@ -175,7 +175,7 @@ <h4 class="trace-steps-title">Trace</h4>
   </main>
 
   <footer class="bottombar">
-    <span>v1.25.0</span>
+    <span>v1.26.0</span>
     <span>·</span>
     <span>MIT</span>
     <span>·</span>

package.json

@@ -1,6 +1,6 @@
 {
   "name": "firewallscope",
-  "version": "1.25.0",
+  "version": "1.26.0",
   "description": "Visual analyzer for iptables, ip6tables, nftables and ufw rulesets — paste the output, see the structure.",
   "scripts": {
     "lint": "eslint 'src/**/*.js' 'test/**/*.js'",

samples/iptables-router-sloppy.txt

@@ -16,6 +16,9 @@
 # -j DROP on the -m limit line: the bucket points the wrong way, so the rule
 # discards the first 25 SYN/s — calm, legitimate visitors — and once the
 # bucket runs dry the real flood sails past it into the ACCEPT below.
+# The SMTP "guard" is the half-fix of that same recipe: someone flipped the
+# matcher to --hashlimit-above (per source, even) but left -j ACCEPT, so it
+# admits only the traffic ABOVE 10/s — quiet mail is dead, floods get in.
 *filter
 :INPUT DROP [0:0]
 :FORWARD ACCEPT [0:0]
@@ -28,6 +31,7 @@
 -A INPUT -p tcp -m tcp --dport 8443 -m limit --limit 10/min -j ACCEPT
 -A INPUT -p tcp -m tcp --dport 80 --tcp-flags FIN,SYN,RST,ACK SYN -m limit --limit 25/sec --limit-burst 100 -j DROP
 -A INPUT -p tcp -m tcp --dport 80 -j ACCEPT
+-A INPUT -p tcp -m tcp --dport 25 -m hashlimit --hashlimit-above 10/sec --hashlimit-mode srcip --hashlimit-name smtpguard -j ACCEPT
 -A INPUT -s 169.254.0.0/16 -j ACCEPT
 -A INPUT -p icmp -j ACCEPT
 COMMIT

src/linter.js

@@ -59,6 +59,7 @@
           detectAdminPortNoRateLimit(chain, table, findings, result.format);
           detectRateLimitNotPerSource(chain, table, findings, result.format);
           detectRateLimitDropInverted(chain, table, findings, result.format);
+          detectRateLimitAcceptInverted(chain, table, findings, result.format);
         }
         detectUnlimitedLog(chain, table, findings, result.format);
         detectLogTcpSequence(chain, table, findings, result.format);
@@ -1691,6 +1692,12 @@
       // a silent drop), where a global bucket is exactly right. Only a
       // drop-the-excess rule is judged for bucket sharing here.
       if (isDrop && underLimitMatch(rule)) continue;
+      // Same courtesy in the other direction: an over-limit matcher on an
+      // ACCEPT admits only the excess — inverted regardless of how the
+      // bucket is keyed (rate-limit-accept-inverted's job). Judging it for
+      // bucket sharing would prescribe `--hashlimit-mode srcip` as the fix
+      // for a rule whose real problem is the verdict.
+      if (!isDrop && overLimitMatch(rule)) continue;
       const how = sharedBucketLimit(rule);
       if (!how) continue;
       const details = isDrop
@@ -1745,6 +1752,18 @@
     return null;
   }
 
+  // The opposite direction: matchers that fire on the traffic ABOVE the
+  // rate. `-m limit` has no over form, so only hashlimit-above and nft's
+  // `limit rate over` land here.
+  function overLimitMatch(rule) {
+    const raw = String(rule.raw || '');
+    if (/-m\s+hashlimit\b/.test(raw) && /--hashlimit-above\b/.test(raw)) {
+      return '`-m hashlimit --hashlimit-above`';
+    }
+    if (/\blimit\s+rate\s+over\b/.test(raw)) return '`limit rate over`';
+    return null;
+  }
+
   function detectRateLimitDropInverted(chain, table, findings, format) {
     if (format === 'ufw') return;
     const rules = chain.rules || [];
@@ -1766,6 +1785,46 @@
     }
   }
 
+  // ── rate-limit-accept-inverted ─────────────────────────────────────
+  // The fourth quadrant of the rate-limit matrix, and the only one that
+  // was still unjudged. Under-limit ACCEPT = a correct throttle (judged
+  // only for bucket sharing); under-limit DROP = rate-limit-drop-inverted;
+  // over-limit DROP = the correct drop-the-excess recipe (judged for
+  // sharing); over-limit ACCEPT = THIS: the rule admits only the traffic
+  // ABOVE the rate. Calm, legitimate traffic never matches and falls
+  // through to whatever sits below — usually the default deny — so the
+  // service is dead on a quiet day and springs to life only under flood.
+  // Typically born of a half-fix: someone flips the tutorial recipe's
+  // matcher to `over` / `--hashlimit-above` but forgets to flip the
+  // verdict (or swaps verdicts while refactoring). Per-source keying does
+  // not save it — a meter `{ ip saddr limit rate over 3/minute } accept`
+  // just inverts per client — and the protocol doesn't matter (admitting
+  // only excess ICMP is equally backwards), so like its DROP sibling this
+  // judges every protocol and never exempts braces. Mutually exclusive
+  // with rate-limit-not-per-source by construction: an over-limit ACCEPT
+  // lands here and only here (the verdict is the bug, not the keying).
+  // Skipped for ufw, whose `limit` verb compiles to a correct recipe.
+  function detectRateLimitAcceptInverted(chain, table, findings, format) {
+    if (format === 'ufw') return;
+    const rules = chain.rules || [];
+    for (let i = 0; i < rules.length; i++) {
+      const rule = rules[i];
+      if (!isAcceptAction(rule)) continue;
+      const how = overLimitMatch(rule);
+      if (!how) continue;
+      findings.push({
+        id: 'rate-limit-accept-inverted',
+        severity: 'warning',
+        table: table.name,
+        tableFamily: table.family || null,
+        chain: chain.name,
+        ruleIdx: i,
+        title: 'Rate-limited ACCEPT admits only the excess',
+        details: `This ACCEPT is gated by ${how}, which matches traffic only while it is ABOVE the rate — calm, legitimate traffic never matches and falls through to the rules below (usually the default deny), so the service is dead on a quiet day and answers only under flood. Flip the direction, not the keying: accept under the limit (\`-m hashlimit --hashlimit-upto … --hashlimit-mode srcip -j ACCEPT\`, nft \`limit rate 10/second accept\`) — or keep the over-limit match but make it a DROP that sheds the excess above a plain ACCEPT.`
+      });
+    }
+  }
+
   // ── mac-based-trust ────────────────────────────────────────────────
   // A MAC address is identification, not authentication: it is broadcast
   // to the whole local segment (ARP/NDP) and forged with one `ip link set