scan rules: Clean code tweaks

- Add static modifier where applicable - Remove boiler plate or useless comments/JavaDoc attributes. - CHANGELOG > Add maintenance note (if there wasn't already one present). - pscanrules > Made resource message methods private again where example alerts have been implemented. Signed-off-by: kingthorin <[email protected]>
zaproxy · Jun 27, 2024 · 6c5ca97 · 6c5ca97
1 parent 5803df8
commit 6c5ca97
Show file tree

Hide file tree

Showing 140 changed files with 338 additions and 663 deletions.
diff --git a/addOns/ascanrules/CHANGELOG.md b/addOns/ascanrules/CHANGELOG.md
@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - The following rules now includes example alert functionality for documentation generation purposes (Issue 6119), as well as now including Alert Tags (OWASP Top 10, WSTG, and updated CWE):
     - Server Side Template Injection
     - Server Side Template Injection (Blind)
+- Maintenance changes.
 
 ### Fixed
 - False positives in the Path Traversal rule.

diff --git a/...ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/BufferOverflowScanRule.java b/...ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/BufferOverflowScanRule.java
@@ -101,7 +101,7 @@ public String getOther() {
     public void scan(HttpMessage msg, String param, String value) {
 
         if (this.isStop()) { // Check if the user stopped things
-            LOGGER.debug("Scanner {} Stopping.", this.getName());
+            LOGGER.debug("Scan rule {} Stopping.", this.getName());
             return; // Stop!
         }
         if (isPage500(getBaseMsg())) // Check to see if the page closed initially
@@ -169,7 +169,7 @@ public int getWascId() {
         return 7;
     }
 
-    private String randomCharacterString(int length) {
+    private static String randomCharacterString(int length) {
         StringBuilder sb1 = new StringBuilder(length + 1);
         int counter = 0;
         int character = 0;

diff --git a/...canrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java b/...canrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java
@@ -282,7 +282,6 @@ public class CommandInjectionScanRule extends AbstractAppParamPlugin
         NIX_BLIND_OS_PAYLOADS.add("|" + insertedCMD + "#");
     }
 
-    // Logger instance
     private static final Logger LOGGER = LogManager.getLogger(CommandInjectionScanRule.class);
 
     // Get WASC Vulnerability description
@@ -366,7 +365,7 @@ public int getRisk() {
         return Alert.RISK_HIGH;
     }
 
-    private String getOtherInfo(TestType testType, String testValue) {
+    private static String getOtherInfo(TestType testType, String testValue) {
         return Constant.messages.getString(
                 MESSAGE_PREFIX + "otherinfo." + testType.getNameKey(), testValue);
     }

diff --git a/...anrules/src/main/java/org/zaproxy/zap/extension/ascanrules/DirectoryBrowsingScanRule.java b/...anrules/src/main/java/org/zaproxy/zap/extension/ascanrules/DirectoryBrowsingScanRule.java
@@ -95,7 +95,7 @@ public String getReference() {
         return Constant.messages.getString(MESSAGE_PREFIX + "refs");
     }
 
-    private void checkIfDirectory(HttpMessage msg) throws URIException {
+    private static void checkIfDirectory(HttpMessage msg) throws URIException {
 
         URI uri = msg.getRequestHeader().getURI();
         uri.setQuery(null);

diff --git a/...canrules/src/main/java/org/zaproxy/zap/extension/ascanrules/ExternalRedirectScanRule.java b/...canrules/src/main/java/org/zaproxy/zap/extension/ascanrules/ExternalRedirectScanRule.java
@@ -147,13 +147,6 @@ public String getReference() {
         return VULN.getReferencesAsString();
     }
 
-    /**
-     * Scan for External Redirect vulnerabilities
-     *
-     * @param msg a request only copy of the original message (the response isn't copied)
-     * @param param the parameter name that need to be exploited
-     * @param value the original parameter value
-     */
     @Override
     public void scan(HttpMessage msg, String param, String value) {
 
@@ -342,7 +335,7 @@ private static boolean isRedirectHost(String value, boolean escaped) throws URIE
      * @param msg the current message where reflected redirection should be check into
      * @return get back the redirection type if exists
      */
-    private int isRedirected(String payload, HttpMessage msg) {
+    private static int isRedirected(String payload, HttpMessage msg) {
 
         // (1) Check if redirection by "Location" header
         // http://en.wikipedia.org/wiki/HTTP_location
@@ -471,7 +464,7 @@ private static boolean isRedirectPresent(Pattern pattern, String value) {
      * @param type the redirection type
      * @return a string representing the reason of this redirection
      */
-    private String getRedirectionReason(int type) {
+    private static String getRedirectionReason(int type) {
         switch (type) {
             case REDIRECT_LOCATION_HEADER:
                 return Constant.messages.getString(MESSAGE_PREFIX + "reason.location.header");
@@ -493,11 +486,6 @@ private String getRedirectionReason(int type) {
         }
     }
 
-    /**
-     * Give back the risk associated to this vulnerability (high)
-     *
-     * @return the risk according to the Alert enum
-     */
     @Override
     public int getRisk() {
         return Alert.RISK_HIGH;
@@ -508,24 +496,14 @@ public Map<String, String> getAlertTags() {
         return ALERT_TAGS;
     }
 
-    /**
-     * http://cwe.mitre.org/data/definitions/601.html
-     *
-     * @return the official CWE id
-     */
     @Override
     public int getCweId() {
-        return 601;
+        return 601; // http://cwe.mitre.org/data/definitions/601.html
     }
 
-    /**
-     * http://projects.webappsec.org/w/page/13246981/URL%20Redirector%20Abuse
-     *
-     * @return the official WASC id
-     */
     @Override
     public int getWascId() {
-        return 38;
+        return 38; // http://projects.webappsec.org/w/page/13246981/URL%20Redirector%20Abuse
     }
 
     @Override

diff --git a/...s/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/FormatStringScanRule.java b/...s/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/FormatStringScanRule.java
@@ -105,19 +105,15 @@ public String getReference() {
         return Constant.messages.getString(MESSAGE_PREFIX + "refs");
     }
 
-    private String getError(char c) {
+    private static String getError(char c) {
         return Constant.messages.getString(MESSAGE_PREFIX + "error" + c);
     }
 
-    /*
-     * This method is called by the active scanner for each GET and POST parameter for every page
-     * @see org.parosproxy.paros.core.scanner.AbstractAppParamPlugin#scan(org.parosproxy.paros.network.HttpMessage, java.lang.String, java.lang.String)
-     */
     @Override
     public void scan(HttpMessage msg, String param, String value) {
 
         if (this.isStop()) { // Check if the user stopped things
-            LOGGER.debug("Scanner {} Stopping.", getName());
+            LOGGER.debug("Scan rule {} Stopping.", getName());
             return; // Stop!
         }
 
@@ -223,7 +219,7 @@ && isPage200(verificationMsg)) {
             // errors.  It is only
             //  used if the GNU and generic C compiler check fails to find a vulnerability.
             if (this.isStop()) { // Check if the user stopped things
-                LOGGER.debug("Scanner {} Stopping.", getName());
+                LOGGER.debug("Scan rule {} Stopping.", getName());
                 return; // Stop!
             }
             StringBuilder sb2 = new StringBuilder();
@@ -276,13 +272,11 @@ public Map<String, String> getAlertTags() {
 
     @Override
     public int getCweId() {
-        // The CWE id
         return 134;
     }
 
     @Override
     public int getWascId() {
-        // The WASC ID
         return 6;
     }
 

diff --git a/...canrules/src/main/java/org/zaproxy/zap/extension/ascanrules/HeartBleedActiveScanRule.java b/...canrules/src/main/java/org/zaproxy/zap/extension/ascanrules/HeartBleedActiveScanRule.java
@@ -51,7 +51,6 @@ public class HeartBleedActiveScanRule extends AbstractHostPlugin
     /** the timeout, which is controlled by the Attack Strength */
     private int timeoutMs = 0;
 
-    /** the logger object */
     private static final Logger LOGGER = LogManager.getLogger(HeartBleedActiveScanRule.class);
 
     /** Prefix for internationalized messages used by this rule */
@@ -868,7 +867,6 @@ public class HeartBleedActiveScanRule extends AbstractHostPlugin
         0x40,
         0x00 // payload length to be sent back by the server.  0x40 0x00 = 16384 in decimal
         // Note: No actual payload sent!
-        // Note: No actual padding sent!
     };
 
     @Override

diff --git a/.../ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/PaddingOracleScanRule.java b/.../ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/PaddingOracleScanRule.java
@@ -267,7 +267,7 @@ private String getEmptyValueResponse(String paramName) throws IOException {
      * @param value the value that need to be checked
      * @return true if it seems to be encrypted
      */
-    private boolean isEncrypted(byte[] value) {
+    private static boolean isEncrypted(byte[] value) {
 
         // Make sure we have a reasonable sized string
         // (encrypted strings tend to be long, and short strings tend to break our numbers)

diff --git a/.../ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/PathTraversalScanRule.java b/.../ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/PathTraversalScanRule.java
@@ -49,9 +49,7 @@
 public class PathTraversalScanRule extends AbstractAppParamPlugin
         implements CommonActiveScanRuleInfo {
 
-    /*
-     * Prefix for internationalised messages used by this rule
-     */
+    // Prefix for internationalised messages used by this rule
     private static final String MESSAGE_PREFIX = "ascanrules.pathtraversal.";
 
     private static final Map<String, String> ALERT_TAGS =
@@ -608,7 +606,7 @@ private boolean sendAndCheckPayload(
         return false;
     }
 
-    private String getContentsToMatch(HttpMessage message) {
+    private static String getContentsToMatch(HttpMessage message) {
         return message.getResponseHeader().isHtml()
                 ? StringEscapeUtils.unescapeHtml4(message.getResponseBody().toString())
                 : message.getResponseHeader().toString() + message.getResponseBody().toString();
@@ -700,7 +698,7 @@ public String match(String contents) {
             return matchWinDirectories(contents);
         }
 
-        private String matchNixDirectories(String contents) {
+        private static String matchNixDirectories(String contents) {
             Pattern procPattern =
                     Pattern.compile("(?:^|\\W)proc(?:\\W|$)", Pattern.CASE_INSENSITIVE);
             Pattern etcPattern = Pattern.compile("(?:^|\\W)etc(?:\\W|$)", Pattern.CASE_INSENSITIVE);
@@ -727,7 +725,7 @@ private String matchNixDirectories(String contents) {
             return null;
         }
 
-        private String matchWinDirectories(String contents) {
+        private static String matchWinDirectories(String contents) {
             if (contents.contains("Windows")
                     && Pattern.compile("Program\\sFiles").matcher(contents).find()) {
                 return "Windows";

diff --git a/...ain/java/org/zaproxy/zap/extension/ascanrules/RemoteCodeExecutionCve20121823ScanRule.java b/...ain/java/org/zaproxy/zap/extension/ascanrules/RemoteCodeExecutionCve20121823ScanRule.java
@@ -54,7 +54,6 @@ public class RemoteCodeExecutionCve20121823ScanRule extends AbstractAppPlugin
      */
     private static final Vulnerability VULN = Vulnerabilities.getDefault().get("wasc_20");
 
-    /** the logger object */
     private static final Logger LOGGER =
             LogManager.getLogger(RemoteCodeExecutionCve20121823ScanRule.class);
 

diff --git a/...anrules/src/main/java/org/zaproxy/zap/extension/ascanrules/RemoteFileIncludeScanRule.java b/...anrules/src/main/java/org/zaproxy/zap/extension/ascanrules/RemoteFileIncludeScanRule.java
@@ -37,7 +37,7 @@
 import org.zaproxy.addon.commonlib.vulnerabilities.Vulnerabilities;
 import org.zaproxy.addon.commonlib.vulnerabilities.Vulnerability;
 
-/** a scanner that looks for Remote File Include vulnerabilities */
+/** a scan rule that looks for Remote File Include vulnerabilities */
 public class RemoteFileIncludeScanRule extends AbstractAppParamPlugin
         implements CommonActiveScanRuleInfo {
 

diff --git a/...rc/main/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureWebInfScanRule.java b/...rc/main/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureWebInfScanRule.java
@@ -46,8 +46,8 @@
 import org.zaproxy.addon.commonlib.vulnerabilities.Vulnerability;
 
 /**
- * a scanner that looks for Java classes disclosed via the WEB-INF folder and that decompiles them
- * to give the Java source code. The scanner also looks for easy pickings in the form of properties
+ * a scan rule that looks for Java classes disclosed via the WEB-INF folder and that decompiles them
+ * to give the Java source code. The rule also looks for easy pickings in the form of properties
  * files loaded by the Java class.
  *
  * @author 70pointer
@@ -270,14 +270,8 @@ private HttpMessage createHttpMessage(URI uri) throws HttpMalformedHeaderExcepti
         return msg;
     }
 
-    /**
-     * gets a candidate URI for a given class path.
-     *
-     * @param classname
-     * @return
-     * @throws URIException
-     */
-    private URI getClassURI(URI hostURI, String classname) throws URIException {
+    /** gets a candidate URI for a given class path. */
+    private static URI getClassURI(URI hostURI, String classname) throws URIException {
         return new URI(
                 hostURI.getScheme()
                         + "://"
@@ -288,7 +282,7 @@ private URI getClassURI(URI hostURI, String classname) throws URIException {
                 false);
     }
 
-    private URI getPropsFileURI(URI hostURI, String propsfilename) throws URIException {
+    private static URI getPropsFileURI(URI hostURI, String propsfilename) throws URIException {
         return new URI(
                 hostURI.getScheme()
                         + "://"

diff --git a/...s/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/Spring4ShellScanRule.java b/...s/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/Spring4ShellScanRule.java
@@ -76,11 +76,11 @@ public String getDescription() {
         return Constant.messages.getString("ascanrules.spring4shell.desc");
     }
 
-    private boolean is400Response(HttpMessage msg) {
+    private static boolean is400Response(HttpMessage msg) {
         return !msg.getResponseHeader().isEmpty() && msg.getResponseHeader().getStatusCode() == 400;
     }
 
-    private void setGetPayload(HttpMessage msg, String payload) throws URIException {
+    private static void setGetPayload(HttpMessage msg, String payload) throws URIException {
         msg.getRequestHeader().setMethod("GET");
         URI uri = msg.getRequestHeader().getURI();
         String query = uri.getEscapedQuery();
@@ -92,7 +92,7 @@ private void setGetPayload(HttpMessage msg, String payload) throws URIException
         uri.setEscapedQuery(query);
     }
 
-    private void setPostPayload(HttpMessage msg, String payload) {
+    private static void setPostPayload(HttpMessage msg, String payload) {
         msg.getRequestHeader().setMethod("POST");
         String body = msg.getRequestBody().toString();
         if (body.isEmpty()

diff --git a/...es/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRule.java b/...es/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRule.java
@@ -194,7 +194,6 @@ public class SqlInjectionHypersonicScanRule extends AbstractAppParamPlugin
                     CommonAlertTag.OWASP_2017_A01_INJECTION,
                     CommonAlertTag.WSTG_V42_INPV_05_SQLI);
 
-    /** for logging. */
     private static final Logger LOGGER = LogManager.getLogger(SqlInjectionHypersonicScanRule.class);
 
     /** The number of seconds used in time-based attacks (i.e. sleep commands). */

diff --git a/...anrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRule.java b/...anrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRule.java
@@ -130,7 +130,6 @@ public class SqlInjectionMsSqlScanRule extends AbstractAppParamPlugin
     private static final double TIME_CORRELATION_ERROR_RANGE = 0.15;
     private static final double TIME_SLOPE_ERROR_RANGE = 0.30;
 
-    /** for logging. */
     private static final Logger LOGGER = LogManager.getLogger(SqlInjectionMsSqlScanRule.class);
 
     private static final Map<String, String> ALERT_TAGS =

diff --git a/...anrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRule.java b/...anrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRule.java
@@ -213,7 +213,6 @@ public class SqlInjectionMySqlScanRule extends AbstractAppParamPlugin
                     CommonAlertTag.OWASP_2017_A01_INJECTION,
                     CommonAlertTag.WSTG_V42_INPV_05_SQLI);
 
-    /** for logging. */
     private static final Logger LOGGER = LogManager.getLogger(SqlInjectionMySqlScanRule.class);
 
     private int timeSleepSeconds = DEFAULT_SLEEP_TIME;

diff --git a/...nrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRule.java b/...nrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRule.java
@@ -149,7 +149,6 @@ public class SqlInjectionOracleScanRule extends AbstractAppParamPlugin
                     CommonAlertTag.OWASP_2017_A01_INJECTION,
                     CommonAlertTag.WSTG_V42_INPV_05_SQLI);
 
-    /** for logging. */
     private static final Logger LOGGER = LogManager.getLogger(SqlInjectionOracleScanRule.class);
 
     @Override

diff --git a/...rules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRule.java b/...rules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRule.java
@@ -191,7 +191,6 @@ public class SqlInjectionPostgreScanRule extends AbstractAppParamPlugin
                     CommonAlertTag.OWASP_2017_A01_INJECTION,
                     CommonAlertTag.WSTG_V42_INPV_05_SQLI);
 
-    /** for logging. */
     private static final Logger LOGGER = LogManager.getLogger(SqlInjectionPostgreScanRule.class);
 
     @Override