Merge pull request #5541 from kingthorin/rules-clean

scan rules: Clean code tweaks

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

@@ -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;

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

@@ -366,7 +366,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);
     }

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

@@ -342,7 +342,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 +471,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");

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

@@ -105,7 +105,7 @@ 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);
     }
 

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

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

@@ -608,7 +608,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 +700,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 +727,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";

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

@@ -277,7 +277,7 @@ private HttpMessage createHttpMessage(URI uri) throws HttpMalformedHeaderExcepti
      * @return
      * @throws URIException
      */
-    private URI getClassURI(URI hostURI, String classname) throws URIException {
+    private static URI getClassURI(URI hostURI, String classname) throws URIException {
         return new URI(
                 hostURI.getScheme()
                         + "://"
@@ -288,7 +288,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()
                         + "://"

addOns/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()

addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/ExternalRedirectScanRuleUnitTest.java

@@ -80,11 +80,11 @@ private enum PayloadHandling {
         CONCAT_PATH
     };
 
-    private NanoServerHandler createHttpRedirectHandler(String path, String header) {
+    private static NanoServerHandler createHttpRedirectHandler(String path, String header) {
         return createHttpRedirectHandler(path, header, PayloadHandling.NEITHER);
     }
 
-    private NanoServerHandler createHttpRedirectHandler(
+    private static NanoServerHandler createHttpRedirectHandler(
             String path, String header, PayloadHandling payloadHandling) {
         return new NanoServerHandler(path) {
             @Override

addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/HiddenFilesScanRuleUnitTest.java

@@ -110,7 +110,7 @@ void checkNoPathsHaveLeadingSlash() {
         }
     }
 
-    private void assertNoLeadingSlash(String message, String path) {
+    private static void assertNoLeadingSlash(String message, String path) {
         assertThat(message.replace(REPLACE_TOKEN, path), !path.startsWith("/"), is(true));
     }
 

addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/XxeScanRuleUnitTest.java

@@ -314,7 +314,7 @@ void shouldAlertOnlyIfCertainTagValuesArePresent()
         assertThat(alert.getConfidence(), equalTo(Alert.CONFIDENCE_MEDIUM));
     }
 
-    private NanoServerHandler createNanoHandler(
+    private static NanoServerHandler createNanoHandler(
             String path, NanoHTTPD.Response.IStatus status, String responseBody) {
         return new NanoServerHandler(path) {
             @Override

addOns/ascanrulesAlpha/CHANGELOG.md

@@ -4,7 +4,8 @@ All notable changes to this add-on will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased
-
+### Changed
+- Maintenance changes.
 
 ## [48] - 2024-09-02
 ### Changed

addOns/ascanrulesAlpha/src/main/java/org/zaproxy/zap/extension/ascanrulesAlpha/ExampleFileActiveScanRule.java

@@ -43,7 +43,8 @@
  *
  * @author psiinon
  */
-public class ExampleFileActiveScanRule extends AbstractAppParamPlugin {
+public class ExampleFileActiveScanRule extends AbstractAppParamPlugin
+        implements CommonActiveScanRuleInfo {
 
     /** Prefix for internationalized messages used by this rule */
     private static final String MESSAGE_PREFIX = "ascanalpha.examplefile.";
@@ -80,7 +81,7 @@ public String getDescription() {
         return Constant.messages.getString(MESSAGE_PREFIX + "desc");
     }
 
-    private String getOtherInfo() {
+    private static String getOtherInfo() {
         return Constant.messages.getString(MESSAGE_PREFIX + "other");
     }
 
@@ -155,14 +156,7 @@ public void scan(HttpMessage msg, String param, String value) {
                 String evidence;
                 if ((evidence = doesResponseContainString(msg.getResponseBody(), attack)) != null) {
                     // Raise an alert
-                    newAlert()
-                            .setConfidence(Alert.CONFIDENCE_MEDIUM)
-                            .setParam(param)
-                            .setAttack(attack)
-                            .setOtherInfo(getOtherInfo())
-                            .setEvidence(evidence)
-                            .setMessage(testMsg)
-                            .raise();
+                    createAlert(param, attack, evidence).setMessage(testMsg).raise();
                     return;
                 }
             }
@@ -194,7 +188,16 @@ private String doesResponseContainString(HttpBody body, String str) {
         return null;
     }
 
-    private List<String> loadFile(String file) {
+    private AlertBuilder createAlert(String param, String attack, String evidence) {
+        return newAlert()
+                .setConfidence(Alert.CONFIDENCE_MEDIUM)
+                .setParam(param)
+                .setAttack(attack)
+                .setOtherInfo(getOtherInfo())
+                .setEvidence(evidence);
+    }
+
+    private static List<String> loadFile(String file) {
         /*
          * ZAP will have already extracted the file from the add-on and put it underneath the 'ZAP home' directory
          */
@@ -244,4 +247,9 @@ public int getWascId() {
         // The WASC ID
         return 0;
     }
+
+    @Override
+    public List<Alert> getExampleAlerts() {
+        return List.of(createAlert("foo", "<SCRIPT>a=/XSS/", "<SCRIPT>a=/XSS/").build());
+    }
 }

addOns/ascanrulesAlpha/src/main/java/org/zaproxy/zap/extension/ascanrulesAlpha/ExampleSimpleActiveScanRule.java

@@ -20,6 +20,7 @@
 package org.zaproxy.zap.extension.ascanrulesAlpha;
 
 import java.io.IOException;
+import java.util.List;
 import java.util.Random;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -39,7 +40,8 @@
  *
  * @author psiinon
  */
-public class ExampleSimpleActiveScanRule extends AbstractAppParamPlugin {
+public class ExampleSimpleActiveScanRule extends AbstractAppParamPlugin
+        implements CommonActiveScanRuleInfo {
 
     // wasc_10 is Denial of Service - well, its just an example ;)
     private static final Vulnerability VULN = Vulnerabilities.getDefault().get("wasc_10");
@@ -59,8 +61,7 @@ public int getId() {
 
     @Override
     public String getName() {
-        // Strip off the "Example Active Scan Rule: " part if implementing a real one ;)
-        return "Example Active Scan Rule: " + VULN.getName();
+        return Constant.messages.getString("ascanalpha.examplesimple.name");
     }
 
     @Override
@@ -118,12 +119,7 @@ public void scan(HttpMessage msg, String param, String value) {
             // For this example we're just going to raise the alert at random!
 
             if (rnd.nextInt(10) == 0) {
-                newAlert()
-                        .setConfidence(Alert.CONFIDENCE_MEDIUM)
-                        .setParam(param)
-                        .setAttack(value)
-                        .setMessage(testMsg)
-                        .raise();
+                createAlert(param, attack).setMessage(testMsg).raise();
                 return;
             }
 
@@ -132,6 +128,10 @@ public void scan(HttpMessage msg, String param, String value) {
         }
     }
 
+    private AlertBuilder createAlert(String param, String attack) {
+        return newAlert().setConfidence(Alert.CONFIDENCE_MEDIUM).setParam(param).setAttack(attack);
+    }
+
     @Override
     public int getRisk() {
         return Alert.RISK_HIGH;
@@ -148,4 +148,9 @@ public int getWascId() {
         // The WASC ID
         return 0;
     }
+
+    @Override
+    public List<Alert> getExampleAlerts() {
+        return List.of(createAlert("foo", "attack").build());
+    }
 }

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

@@ -9,14 +9,14 @@
 <H1>Active Scan Rules - Alpha</H1>
 The following alpha status active scan rules are included in this add-on:
 
-<H2>An example active scan rule which loads data from a file</H2>
+<H2 id="id-60101">An example active scan rule which loads data from a file</H2>
 This implements an example active scan rule that loads strings from a file that the user can edit.<br>
 For more details see: 
 <a href="https://www.zaproxy.org/blog/2014-04-30-hacking-zap-4-active-scan-rules/">Hacking ZAP Part 4: Active Scan Rules</a>.
 <p>
 Latest code: <a href="https://github.com/zaproxy/zap-extensions/blob/main/addOns/ascanrulesAlpha/src/main/java/org/zaproxy/zap/extension/ascanrulesAlpha/ExampleFileActiveScanRule.java">ExampleFileActiveScanRule.java</a>
 
-<H2>Example Active Scan Rule: Denial of Service</H2>
+<H2 id="id-60100">Example Active Scan Rule: Denial of Service</H2>
 This implements a very simple example active scan rule.<br>
 For more details see: 
 <a href="https://www.zaproxy.org/blog/2014-04-30-hacking-zap-4-active-scan-rules/">Hacking ZAP Part 4: Active Scan Rules</a>.

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

@@ -6,6 +6,8 @@ ascanalpha.examplefile.other = This is for information that doesnt fit in any of
 ascanalpha.examplefile.refs = https://www.zaproxy.org/blog/2014-04-30-hacking-zap-4-active-scan-rules/
 ascanalpha.examplefile.soln = A general description of how to solve the problem.
 
+ascanalpha.examplesimple.name = Example Active Scan Rule: Denial of Service
+
 #ascanalpha.ldapinjection.alert.attack=[{0}] field [{1}] set to [{2}]
 ascanalpha.ldapinjection.alert.attack = parameter [{0}] set to [{1}]
 #ascanalpha.ldapinjection.alert.extrainfo=[{0}] field [{1}] on [{2}] [{3}] may be vulnerable to LDAP injection, using an attack with LDAP meta-characters [{4}], yielding known [{5}] error message [{6}], which was not present in the original response.

addOns/ascanrulesAlpha/src/test/java/org/zaproxy/zap/extension/ascanrulesAlpha/ExampleFileActiveScanRuleUnitTest.java

@@ -0,0 +1,53 @@
+/*
+ * Zed Attack Proxy (ZAP) and its related class files.
+ *
+ * ZAP is an HTTP/HTTPS proxy for assessing web application security.
+ *
+ * Copyright 2024 The ZAP Development Team
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.zaproxy.zap.extension.ascanrulesAlpha;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
+
+import java.util.List;
+import org.junit.jupiter.api.Test;
+import org.parosproxy.paros.core.scanner.Alert;
+
+class ExampleFileActiveScanRuleUnitTest extends ActiveScannerTest<ExampleFileActiveScanRule> {
+
+    @Override
+    protected ExampleFileActiveScanRule createScanner() {
+        return new ExampleFileActiveScanRule();
+    }
+
+    @Test
+    void shouldHaveExpectedExample() {
+        // Given / When
+        List<Alert> alerts = rule.getExampleAlerts();
+        // Then
+        assertThat(alerts, hasSize(1));
+        Alert alert = alerts.get(0);
+        assertThat(alert.getParam(), is(equalTo("foo")));
+    }
+
+    @Test
+    @Override
+    public void shouldHaveValidReferences() {
+        super.shouldHaveValidReferences();
+    }
+}