Merge remote-tracking branch 'origin/topic/vern/http-sqli-replacement'

* origin/topic/vern/http-sqli-replacement:
  site/local: Switch to detect-sql-injection
  Add a revised script for detecting HTTP SQL injection, deprecate original

CHANGES

@@ -1,3 +1,9 @@
+8.0.0-dev.146 | 2025-05-20 16:25:58 +0200
+
+  * site/local: Switch to detect-sql-injection (Arne Welzel, Corelight)
+
+  * Add a revised script for detecting HTTP SQL injection, deprecate original (Vern Paxson, Corelight)
+
 8.0.0-dev.142 | 2025-05-19 12:32:56 -0700
 
   * EventTrace: Fix operator==() UBSAN downcast error (Arne Welzel, Corelight)

NEWS

@@ -53,6 +53,18 @@ Removed Functionality
 Deprecated Functionality
 ------------------------
 
+- The ``protocols/http/detect-sqli.zeek`` script has been deprecated in favor of a
+  new ``protocols/http/detect-sql-injection.zeek`` script to switch from the victim
+  host being placed into the ``src`` field of a notice to instead use ``dst``.
+  The attacker host is now placed into ``src``. Further, notices hold the first
+  sampled connection uid.
+
+  Note that the ``Notice::Type`` enumeration names remain the same. You can determine
+  which script was used by the presence of populated ``uid`` and ``dst`` fields in the
+  ``notice.log`` entries.
+
+  The replacement script doesn't populate the ``email_body_sections`` anymore either.
+
 Zeek 7.2.0
 ==========
 

VERSION

@@ -1 +1 @@
-8.0.0-dev.142
+8.0.0-dev.146

scripts/policy/protocols/http/detect-sql-injection.zeek

@@ -0,0 +1,142 @@
+##! SQL injection attack detection in HTTP.
+
+## The script annotates the notices it generates with an associated $uid
+## connection identifier; always provides an attacker IP address in the
+## $src field; and always provides a victim IP address in the $dst field.
+
+@load base/frameworks/notice
+@load base/frameworks/sumstats
+@load base/protocols/http
+
+module HTTP;
+
+export {
+	redef enum Notice::Type += {
+		## Indicates that a host performing SQL injection attacks was
+		## detected.
+		SQL_Injection_Attacker,
+
+		## Indicates that a host was seen to have SQL injection attacks
+		## against it.  This is tracked by IP address as opposed to
+		## hostname.
+		SQL_Injection_Victim,
+	};
+
+	redef enum Tags += {
+		## Indicator of a URI based SQL injection attack.
+		URI_SQLI,
+	};
+
+	## Defines the threshold that determines if an SQL injection attack
+	## is ongoing based on the number of requests that appear to be SQL
+	## injection attacks.
+	const sqli_requests_threshold: double = 50.0 &redef;
+
+	## Interval at which to watch for the
+	## :zeek:id:`HTTP::sqli_requests_threshold` variable to be crossed.
+	## At the end of each interval the counter is reset.
+	const sqli_requests_interval = 5 min &redef;
+
+	## Regular expression is used to match URI based SQL injections.
+	const match_sql_injection_uri =
+		  /[\?&][^[:blank:]\x00-\x1f\|\+]+?=[\-[:alnum:]%]+([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/)*'?([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/|\)?;)+.*?(having|union|exec|select|delete|drop|declare|create|insert)([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/)+/i
+		| /[\?&][^[:blank:]\x00-\x1f\|\+]+?=[\-0-9%]+([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/)*'?([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/|\)?;)+(x?or|n?and)([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/)+'?(([^a-zA-Z&]+)?=|exists)/i
+		| /[\?&][^[:blank:]\x00-\x1f\+]+?=[\-0-9%]*([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/)*'([[:blank:]\x00-\x1f]|\/\*.*?\*\/)*(-|=|\+|\|\|)([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/)*([0-9]|\(?convert|cast)/i
+		| /[\?&][^[:blank:]\x00-\x1f\|\+]+?=([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/)*'([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/|;)*(x?or|n?and|having|union|exec|select|delete|drop|declare|create|regexp|insert)([[:blank:]\x00-\x1f\+]|\/\*.*?\*\/|[\[(])+[a-zA-Z&]{2,}/i
+		| /[\?&][^[:blank:]\x00-\x1f\+]+?=[^\.]*?(char|ascii|substring|truncate|version|length)\(/i
+		| /\/\*![[:digit:]]{5}.*?\*\// &redef;
+
+	## A hook that can be used to prevent specific requests from being counted
+	## as an injection attempt.  Use a 'break' statement to exit the hook
+	## early and ignore the request.
+	global HTTP::sqli_policy: hook(c: connection, method: string, unescaped_URI: string);
+}
+
+redef record SumStats::Observation += {
+	uid: string &optional;
+};
+
+event zeek_init() &priority=3
+	{
+	# Add filters to the metrics so that the metrics framework knows how to
+	# determine when it looks like an actual attack and how to respond when
+	# thresholds are crossed.
+	local r1 = SumStats::Reducer(
+		$stream="http.sqli.attacker",
+		$apply=set(SumStats::SUM, SumStats::SAMPLE),
+		$num_samples=1
+	);
+	local r2 = SumStats::Reducer(
+		$stream="http.sqli.victim",
+		$apply=set(SumStats::SUM, SumStats::SAMPLE),
+		$num_samples=1
+	);
+
+	SumStats::create([
+		$name="detect-sqli-attackers",
+		$epoch=sqli_requests_interval,
+		$reducers=set(r1),
+		$threshold_val(key: SumStats::Key, result: SumStats::Result) =
+			{
+			return result["http.sqli.attacker"]$sum;
+			},
+		$threshold=sqli_requests_threshold,
+		$threshold_crossed(key: SumStats::Key, result: SumStats::Result) =
+			{
+			local r = result["http.sqli.attacker"];
+			local dst = to_addr(r$samples[0]$str);
+			local uid = r$samples[0]$uid;
+			NOTICE([$note=SQL_Injection_Attacker,
+				$msg="An SQL injection attacker was discovered!",
+				$uid=uid,
+				$src=key$host,
+				$dst=dst,
+				$identifier=cat(key$host)]);
+			}
+	]);
+
+	SumStats::create([
+		$name="detect-sqli-victims",
+		$epoch=sqli_requests_interval,
+		$reducers=set(r2),
+		$threshold_val(key: SumStats::Key, result: SumStats::Result) =
+			{
+			return result["http.sqli.victim"]$sum;
+			},
+		$threshold=sqli_requests_threshold,
+		$threshold_crossed(key: SumStats::Key, result: SumStats::Result) =
+			{
+			local r = result["http.sqli.victim"];
+			local src = to_addr(r$samples[0]$str);
+			local uid = r$samples[0]$uid;
+			NOTICE([$note=SQL_Injection_Victim,
+				$msg="An SQL injection victim was discovered!",
+				$uid=uid,
+				$src=src,
+				$dst=key$host,
+				$identifier=cat(key$host)]);
+			}
+	]);
+	}
+
+event http_request(c: connection, method: string, original_URI: string,
+                   unescaped_URI: string, version: string) &priority=3
+	{
+	if ( ! hook HTTP::sqli_policy(c, method, unescaped_URI) )
+		return;
+
+	if ( match_sql_injection_uri !in unescaped_URI )
+		return;
+
+	add c$http$tags[URI_SQLI];
+
+	local id = c$id;
+	local orig = id$orig_h;
+	local resp = id$resp_h;
+	local uid = c$uid;
+
+	SumStats::observe("http.sqli.attacker", SumStats::Key($host=orig),
+		SumStats::Observation($str=fmt("%s", resp), $uid=c$uid));
+	SumStats::observe("http.sqli.victim", SumStats::Key($host=resp),
+		SumStats::Observation($str=fmt("%s", orig), $uid=c$uid));
+	}

scripts/policy/protocols/http/detect-sqli.zeek

@@ -1,5 +1,20 @@
 ##! SQL injection attack detection in HTTP.
 
+## This package is deprecated in favor of detect-sql-injection.zeek.
+##
+## The replacement script annotates the notices it generates with
+## an associated $uid connection identifier; always provides an attacker
+## IP address in the $src field; and always provides a victim IP address
+## in the $dst field. The notices generated by this script, on the other
+## hand, lack a $uid identifier, and do not provide $dst information.
+## In addition, for SQL_Injection_Victim notices, this script provides the
+## victim's IP address in the $src field, which some find counter-intuitive.
+##
+## In addition, the replacement script removes support for generating
+## Notice emails.
+
+@deprecated "Remove in v8.1: Switch to the improved detect-sql-injection script"
+
 @load base/frameworks/notice
 @load base/frameworks/sumstats
 @load base/protocols/http

scripts/site/local.zeek

@@ -74,7 +74,7 @@ redef digest_salt = "Please change this value.";
 @load protocols/ssh/interesting-hostnames
 
 # Detect SQL injection attacks.
-@load protocols/http/detect-sqli
+@load protocols/http/detect-sql-injection
 
 #### Network File Handling ####
 

scripts/test-all-policy.zeek

@@ -119,7 +119,8 @@
 @load protocols/ftp/detect-bruteforcing.zeek
 @load protocols/ftp/detect.zeek
 @load protocols/ftp/software.zeek
-@load protocols/http/detect-sqli.zeek
+# @load protocols/http/detect-sqli.zeek
+@load protocols/http/detect-sql-injection.zeek
 @load protocols/http/detect-webapps.zeek
 @load protocols/http/header-names.zeek
 @load protocols/http/software-browser-plugins.zeek

scripts/zeekygen/__load__.zeek

@@ -17,6 +17,9 @@
 @load policy/misc/dump-events.zeek
 @load policy/protocols/conn/speculative-service.zeek
 
+# Remove in v8.1: This script is deprecated and conflicts with detect-sql-injection.zeek
+# @load policy/protocols/http/detect-sqli.zeek
+
 @if ( have_spicy() )
 # Loading this messes up documentation of some elements defined elsewhere.
 # @load frameworks/spicy/record-spicy-batch.zeek

testing/btest/Baseline/coverage.bare-mode-errors/errors

@@ -1,2 +1,3 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
 ### NOTE: This file has been sorted with diff-sort.
+warning in <...>/detect-sqli.zeek, line 16: deprecated script loaded from command line arguments "Remove in v8.1: Switch to the improved detect-sql-injection script"

testing/btest/Baseline/scripts.base.protocols.krb.enc_part/output

@@ -1,3 +1,4 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
 as_response, CHhAvVGS1DHFjwGM9
 [kvno=<uninitialized>, cipher=18, ciphertext=\xc0\xe9\xbc\x01\x95\x9d\x9e\x9f\x9f\xeeY\x93O\xb4\xf1-W\xfb\x18\x90\xbb\x0d\x86\xa8\xd3\xc3\xc8\x8f\xfa\x8a\x95:\xbd\x0a\x0a\x0c\x1e<\xf3gb\xe8\xaf\x14\xc6O\xca\x04%L\x93+\xa0C&\xcb\xc9\xe7\xe0\x13c\xb6\xee\xa0\xbf\xc1\x14]\x03+\x04\xdc\x13"\x99O\xbb\xce\xf4`\x18oS\x96\x08\x19\xa0\xd3\x7f\x05\x07\xe4\x9a6\xa4K\x8ae]\xc7\x02,k\xabv\x07A$v\x81\xba\x83?\x07\xaa\x0a\x85\x89a[\x89+\xbe\xf7\xe5d&Z\x9c\x87\xee=a\xef\xe4\x18\x1fp\x95\x0f=K\xa2&\xb3s5\xc1\xefTx\x9b\xc8\xf3\x82G\x92\xa6\xa9\x8a\xb6\xc2E\xaf\xe8\xad\xec\x16\x10,\xd5\xb6\xf5\xc4\x84\xfc\xf5\xb21\x13\xb3\x06\xf8/\xf7sY\xf5T.Ou\xcf\xf2*2\xb6<\x1d`Y\xe3\xd8\x88iLA\x07!(@.}}\xe9 ;\x90\xd3\xe0\xd2\xeb&\x02\xaa\xc9\xd3\xcd\xe6ba\x0d|-\xdf\x88\xf3\xd3\xb9\xe4S\xd4\xd7\x04\xbc\x96p\x17YS\x15-\xf9\xc8\x9b\xfd\x00<\xc1C]
 as_response, ClEkJM2Vm5giqnMf4h

testing/btest/Baseline/scripts.policy.protocols.http.sql-injection-plus-dvwa2/http.log.cut

@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+uid	method	host	uri	tags
+ClEkJM2Vm5giqnMf4h	GET	192.168.111.154	/dvwa/vulnerabilities/sqli/?id=1'+OR+'1'='1&Submit=Submit	HTTP::URI_SQLI
+C4J4Th3PJpwUYZZ6gc	GET	192.168.111.154	/dvwa/vulnerabilities/sqli/?id=1'+UNION+SELECT+NULL,+version()+#&Submit=Submit	HTTP::URI_SQLI
+CtPZjS20MLrsMUOJi2	GET	192.168.111.154	/dvwa/vulnerabilities/sqli/?id=2'+OR+'2'='2&Submit=Submit	HTTP::URI_SQLI

testing/btest/Baseline/scripts.policy.protocols.http.sql-injection-plus-dvwa2/notice.log

@@ -0,0 +1,12 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	notice
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	fuid	file_mime_type	file_desc	proto	note	msg	sub	src	dst	p	n	peer_descr	actions	email_dest	suppress_for	remote_location.country_code	remote_location.region	remote_location.city	remote_location.latitude	remote_location.longitude
+#types	time	string	addr	port	addr	port	string	string	string	enum	enum	string	string	addr	addr	port	count	string	set[enum]	set[string]	interval	string	string	string	double	double
+XXXXXXXXXX.XXXXXX	CtPZjS20MLrsMUOJi2	-	-	-	-	-	-	-	-	HTTP::SQL_Injection_Attacker	An SQL injection attacker was discovered!	-	192.168.111.148	192.168.111.154	-	-	-	Notice::ACTION_LOG	(empty)	3600.000000	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CtPZjS20MLrsMUOJi2	-	-	-	-	-	-	-	-	HTTP::SQL_Injection_Victim	An SQL injection victim was discovered!	-	192.168.111.148	192.168.111.154	-	-	-	Notice::ACTION_LOG	(empty)	3600.000000	-	-	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.policy.protocols.http.sql-injection-plus-dvwa2/output

@@ -0,0 +1,4 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ClEkJM2Vm5giqnMf4h, [orig_h=192.168.111.148, orig_p=53796/tcp, resp_h=192.168.111.154, resp_p=80/tcp, proto=6], {\x0a\x09HTTP::URI_SQLI\x0a}
+C4J4Th3PJpwUYZZ6gc, [orig_h=192.168.111.148, orig_p=57524/tcp, resp_h=192.168.111.154, resp_p=80/tcp, proto=6], {\x0a\x09HTTP::URI_SQLI\x0a}
+CtPZjS20MLrsMUOJi2, [orig_h=192.168.111.148, orig_p=40112/tcp, resp_h=192.168.111.154, resp_p=80/tcp, proto=6], {\x0a\x09HTTP::URI_SQLI\x0a}

testing/btest/scripts/policy/protocols/http/sql-injection-plus-dvwa.zeek

@@ -4,6 +4,7 @@
 # @TEST-EXEC: btest-diff http.log.cut
 
 @load base/protocols/http
+# Remove in v8.1: Remove this test when detect-sqli is gone sql-injection-plus-dvwa2.zeek tests detect-sql-injection.
 @load protocols/http/detect-sqli
 
 event connection_state_remove(c: connection)

testing/btest/scripts/policy/protocols/http/sql-injection-plus-dvwa2.zeek

@@ -0,0 +1,18 @@
+# A version of sql-injection-plus-dvwa.zeek that uses its replacement script.
+#
+# @TEST-EXEC: zeek -C -r $TRACES/http/cooper-grill-dvwa.pcapng -b %INPUT >output
+# @TEST-EXEC: btest-diff output
+# @TEST-EXEC: btest-diff notice.log
+# @TEST-EXEC: zeek-cut -m uid method host uri tags < http.log > http.log.cut
+# @TEST-EXEC: btest-diff http.log.cut
+
+@load base/protocols/http
+@load protocols/http/detect-sql-injection
+
+redef HTTP::sqli_requests_threshold = 3;
+
+event connection_state_remove(c: connection)
+	{
+	if ( c?$http )
+		print c$uid, c$id, cat(c$http$tags);
+	}