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]>

Author: kingthorin

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.

addOns/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("Stopping scan rule \"{}\".", this.getName());
             return; // Stop!
         }
         if (isPage500(getBaseMsg())) // Check to see if the page closed initially
@@ -159,17 +159,15 @@ public Map<String, String> getAlertTags() {
 
     @Override
     public int getCweId() {
-        // The CWE id
         return 120;
     }
 
     @Override
     public int getWascId() {
-        // The WASC ID
         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;

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CodeInjectionScanRule.java

@@ -155,7 +155,6 @@ public void init() {
     @Override
     public void scan(HttpMessage msg, String paramName, String value) {
 
-        // Begin scan rule execution
         LOGGER.debug(
                 "Checking [{}][{}], parameter [{}] for Dynamic Code Injection Vulnerabilities",
                 msg.getRequestHeader().getMethod(),

addOns/ascanrules/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);
     }
@@ -405,7 +404,6 @@ int getTimeSleep() {
     @Override
     public void scan(HttpMessage msg, String paramName, String value) {
 
-        // Begin scan rule execution
         LOGGER.debug(
                 "Checking [{}][{}], parameter [{}] for OS Command Injection Vulnerabilities",
                 msg.getRequestHeader().getMethod(),

addOns/ascanrules/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);

addOns/ascanrules/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; // CWE-601: URL Redirection to Untrusted Site ('Open Redirect')
     }
 
-    /**
-     * http://projects.webappsec.org/w/page/13246981/URL%20Redirector%20Abuse
-     *
-     * @return the official WASC id
-     */
     @Override
     public int getWascId() {
-        return 38;
+        return 38; // URL Redirector Abuse
     }
 
     @Override

addOns/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("Stopping scan rule \"{}\".", 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("Stopping scan rule \"{}\".", getName());
                 return; // Stop!
             }
             StringBuilder sb2 = new StringBuilder();
@@ -276,14 +272,12 @@ public Map<String, String> getAlertTags() {
 
     @Override
     public int getCweId() {
-        // The CWE id
-        return 134;
+        return 134; // CWE-134: Use of Externally-Controlled Format String
     }
 
     @Override
     public int getWascId() {
-        // The WASC ID
-        return 6;
+        return 6; // Format String
     }
 
     private AlertBuilder createBaseAlert(

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/GetForPostScanRule.java

@@ -88,7 +88,7 @@ public void scan() {
         // Check if the user stopped things. One request per URL so check before
         // sending the request
         if (isStop()) {
-            LOGGER.debug("Scan rule {} Stopping.", getName());
+            LOGGER.debug("Stopping scan rule \"{}\".", getName());
             return;
         }
 

addOns/ascanrules/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

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/HiddenFilesScanRule.java

@@ -113,7 +113,7 @@ public void scan() {
         for (HiddenFile file : hfList) {
 
             if (isStop()) {
-                LOGGER.debug("Scan rule {} stopping.", getName());
+                LOGGER.debug("Stopping scan rule \"{}\".", getName());
                 return;
             }