ascanrules: SQLi MsSQL rename scan rule (all timing based)

Signed-off-by: kingthorin <[email protected]>

Author: kingthorin

addOns/ascanrules/CHANGELOG.md

@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Changed
 - Maintenance changes.
 - Depends on an updated version of the Common Library add-on.
+- The SQL Injection - MsSQL scan rule and alerts have been renamed to clarify that they're timing based (Issue 7341).
 
 ### Added
 - Rules (as applicable) have been tagged in relation to HIPAA and PCI DSS.

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRule.java renamed to addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlTimingScanRule.java

@@ -44,17 +44,17 @@
 import org.zaproxy.zap.model.TechSet;
 
 /**
- * The SqlInjectionMsSqlScanRule identifies MsSQL specific SQL Injection vulnerabilities using MsSQL
- * specific syntax. If it doesn't use MsSQL specific syntax, it belongs in the generic SQLInjection
- * class! Note the ordering of checks, for efficiency is : 1) Error based (N/A) 2) Boolean Based
- * (N/A - uses standard syntax) 3) UNION based (N/A - uses standard syntax) 4) Stacked (N/A - uses
- * standard syntax) 5) Blind/Time Based (Yes - uses specific syntax)
+ * The SqlInjectionMsSqlTimingScanRule identifies MsSQL specific SQL Injection vulnerabilities using
+ * MsSQL specific syntax. If it doesn't use MsSQL specific syntax, it belongs in the generic
+ * SQLInjection class! Note the ordering of checks, for efficiency is : 1) Error based (N/A) 2)
+ * Boolean Based (N/A - uses standard syntax) 3) UNION based (N/A - uses standard syntax) 4) Stacked
+ * (N/A - uses standard syntax) 5) Blind/Time Based (Yes - uses specific syntax)
  *
  * <p>See the following for some great MySQL specific tricks which could be integrated here
  * http://www.websec.ca/kb/sql_injection#MSSQL_Stacked_Queries
  * http://pentestmonkey.net/cheat-sheet/sql-injection/mssql-sql-injection-cheat-sheet
  */
-public class SqlInjectionMsSqlScanRule extends AbstractAppParamPlugin
+public class SqlInjectionMsSqlTimingScanRule extends AbstractAppParamPlugin
         implements CommonActiveScanRuleInfo {
 
     /** MSSQL one-line comment */
@@ -134,7 +134,8 @@ public class SqlInjectionMsSqlScanRule extends AbstractAppParamPlugin
     private static final double TIME_SLOPE_ERROR_RANGE = 0.30;
 
     /** for logging. */
-    private static final Logger LOGGER = LogManager.getLogger(SqlInjectionMsSqlScanRule.class);
+    private static final Logger LOGGER =
+            LogManager.getLogger(SqlInjectionMsSqlTimingScanRule.class);
 
     private static final Map<String, String> ALERT_TAGS;
 
@@ -146,7 +147,8 @@ public class SqlInjectionMsSqlScanRule extends AbstractAppParamPlugin
                                 CommonAlertTag.OWASP_2017_A01_INJECTION,
                                 CommonAlertTag.WSTG_V42_INPV_05_SQLI,
                                 CommonAlertTag.HIPAA,
-                                CommonAlertTag.PCI_DSS));
+                                CommonAlertTag.PCI_DSS,
+                                CommonAlertTag.TEST_TIMING));
         alertTags.put(PolicyTag.DEV_FULL.getTag(), "");
         alertTags.put(PolicyTag.QA_STD.getTag(), "");
         alertTags.put(PolicyTag.QA_FULL.getTag(), "");

addOns/ascanrules/src/main/javahelp/org/zaproxy/zap/extension/ascanrules/resources/help/contents/ascanrules.html

@@ -365,7 +365,7 @@ <H2 id="id-40020">SQL Injection - Hypersonic (Time Based)</H2>
 <br>
 Alert ID: <a href="https://www.zaproxy.org/docs/alerts/40020/">40020</a>.
 
-<H2 id="id-40027">SQL Injection - MsSQL</H2>
+<H2 id="id-40027">SQL Injection - MsSQL (Time Based)</H2>
 This active scan rule attempts to inject MsSQL specific sleep commands into parameter values and analyzes the server's response time to see if the sleep is effectively executed on the server (indicating a successful SQL injection attack).
 <p>
 Latest code: <a href="https://github.com/zaproxy/zap-extensions/blob/main/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRule.java">SqlInjectionMsSqlScanRule.java</a>

addOns/ascanrules/src/main/resources/org/zaproxy/zap/extension/ascanrules/resources/Messages.properties

@@ -182,7 +182,7 @@ ascanrules.sqlinjection.authbypass.name = SQL Injection - Authentication Bypass
 ascanrules.sqlinjection.desc = SQL injection may be possible.
 ascanrules.sqlinjection.hypersonic.name = SQL Injection - Hypersonic SQL
 ascanrules.sqlinjection.mssql.alert.timebased.extrainfo = The query time is controllable using parameter value [{0}], which caused the request to take [{1}] milliseconds, when the original unmodified query with value [{2}] took [{3}] milliseconds.
-ascanrules.sqlinjection.mssql.name = SQL Injection - MsSQL
+ascanrules.sqlinjection.mssql.name = SQL Injection - MsSQL (Time Based)
 ascanrules.sqlinjection.mysql.name = SQL Injection - MySQL
 ascanrules.sqlinjection.name = SQL Injection
 ascanrules.sqlinjection.oracle.name = SQL Injection - Oracle

addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRuleUnitTest.java renamed to addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlTimingScanRuleUnitTest.java

@@ -38,12 +38,13 @@
 import org.zaproxy.zap.model.TechSet;
 import org.zaproxy.zap.testutils.NanoServerHandler;
 
-/** Unit test for {@link SqlInjectionMsSqlScanRule}. */
-class SqlInjectionMsSqlScanRuleUnitTest extends ActiveScannerTest<SqlInjectionMsSqlScanRule> {
+/** Unit test for {@link SqlInjectionMsSqlTimingScanRule}. */
+class SqlInjectionMsSqlTimingScanRuleUnitTest
+        extends ActiveScannerTest<SqlInjectionMsSqlTimingScanRule> {
 
     @Override
-    protected SqlInjectionMsSqlScanRule createScanner() {
-        return new SqlInjectionMsSqlScanRule();
+    protected SqlInjectionMsSqlTimingScanRule createScanner() {
+        return new SqlInjectionMsSqlTimingScanRule();
     }
 
     @Test
@@ -150,7 +151,7 @@ void shouldReturnExpectedMappings() {
         // Then
         assertThat(cwe, is(equalTo(89)));
         assertThat(wasc, is(equalTo(19)));
-        assertThat(tags.size(), is(equalTo(10)));
+        assertThat(tags.size(), is(equalTo(11)));
         assertThat(
                 tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()),
                 is(equalTo(true)));
@@ -161,6 +162,7 @@ void shouldReturnExpectedMappings() {
                 tags.containsKey(CommonAlertTag.WSTG_V42_INPV_05_SQLI.getTag()), is(equalTo(true)));
         assertThat(tags.containsKey(CommonAlertTag.HIPAA.getTag()), is(equalTo(true)));
         assertThat(tags.containsKey(CommonAlertTag.PCI_DSS.getTag()), is(equalTo(true)));
+        assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true)));
         assertThat(tags.containsKey(PolicyTag.DEV_FULL.getTag()), is(equalTo(true)));
         assertThat(tags.containsKey(PolicyTag.QA_STD.getTag()), is(equalTo(true)));
         assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true)));