ascanrulesBeta: More example alerts

- CHANGELOG > Add notes.
- Scan rules > Add example alert functionality (6119).
- Unit tests > Assert the new example alerts.

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

Author: kingthorin

addOns/ascanrulesBeta/CHANGELOG.md

@@ -4,8 +4,14 @@ 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
+- The following scan rules now include example alert functionality for documentation generation purposes (Issue 6119):
+    - Expression Language Injection
+    - Cookie Slack Detector
+
 ### Fixed
 - Potential false positives in the Source Code Disclosure - File Inclusion scan rule when responses are empty or the original message resulted in an error to start with (Issue 8517).
+- A spacing/punctuation issue in the Cookie Slack Detector scan rule, whereby the Other Info field would not have a space after colons and before lists of cookie names.
 
 ## [54] - 2024-07-22
 ### Changed

addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ExpressionLanguageInjectionScanRule.java

@@ -21,6 +21,7 @@
 
 import java.io.IOException;
 import java.net.UnknownHostException;
+import java.util.List;
 import java.util.Map;
 import java.util.Random;
 import org.apache.commons.httpclient.URIException;
@@ -167,13 +168,7 @@ public void scan(HttpMessage msg, String paramName, String value) {
                         paramName,
                         payload);
 
-                newAlert()
-                        .setConfidence(Alert.CONFIDENCE_MEDIUM)
-                        .setParam(paramName)
-                        .setAttack(payload)
-                        .setEvidence(addedString)
-                        .setMessage(msg)
-                        .raise();
+                createAlert(paramName, payload, addedString).setMessage(msg).raise();
             }
 
         } catch (IOException ex) {
@@ -186,4 +181,17 @@ public void scan(HttpMessage msg, String paramName, String value) {
                     ex);
         }
     }
+
+    private AlertBuilder createAlert(String paramName, String attack, String evidence) {
+        return newAlert()
+                .setConfidence(Alert.CONFIDENCE_MEDIUM)
+                .setParam(paramName)
+                .setAttack(attack)
+                .setEvidence(evidence);
+    }
+
+    @Override
+    public List<Alert> getExampleAlerts() {
+        return List.of(createAlert("foo", "${719117+853088}", "1572205").build());
+    }
 }

addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/SlackerCookieScanRule.java

@@ -22,9 +22,11 @@
 import java.io.IOException;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.TreeSet;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -203,12 +205,11 @@ private void raiseAlert(
 
         int riskLevel = calculateRisk(cookiesThatDoNOTMakeADifference, otherInfoBuff);
 
-        newAlert()
-                .setRisk(riskLevel)
-                .setConfidence(Alert.CONFIDENCE_LOW)
-                .setOtherInfo(otherInfoBuff.toString())
-                .setMessage(msg)
-                .raise();
+        createAlert(riskLevel, otherInfoBuff.toString()).setMessage(msg).raise();
+    }
+
+    private AlertBuilder createAlert(int risk, String otherInfo) {
+        return newAlert().setRisk(risk).setConfidence(Alert.CONFIDENCE_LOW).setOtherInfo(otherInfo);
     }
 
     private StringBuilder createOtherInfoText(
@@ -334,4 +335,17 @@ public int getWascId() {
     public Map<String, String> getAlertTags() {
         return ALERT_TAGS;
     }
+
+    @Override
+    public List<Alert> getExampleAlerts() {
+        SortedSet<String> impacting = new TreeSet<>();
+        impacting.add("foo");
+        impacting.add("bar");
+
+        return List.of(
+                createAlert(
+                                Alert.RISK_INFO,
+                                createOtherInfoText(Set.of("oops"), impacting).toString())
+                        .build());
+    }
 }

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

@@ -9,8 +9,8 @@ ascanbeta.backupfiledisclosure.otherinfo = A backup of [{0}] is available at [{1
 ascanbeta.backupfiledisclosure.refs = https://cwe.mitre.org/data/definitions/530.html\nhttps://owasp.org/www-project-web-security-testing-guide/v41/4-Web_Application_Security_Testing/02-Configuration_and_Deployment_Management_Testing/04-Review_Old_Backup_and_Unreferenced_Files_for_Sensitive_Information.html
 ascanbeta.backupfiledisclosure.soln = Do not edit files in-situ on the web server, and ensure that un-necessary files (including hidden files) are removed from the web server.
 
-ascanbeta.cookieslack.affect.response.no = These cookies did NOT affect the response:
-ascanbeta.cookieslack.affect.response.yes = These cookies affected the response:
+ascanbeta.cookieslack.affect.response.no = These cookies did NOT affect the response: 
+ascanbeta.cookieslack.affect.response.yes = These cookies affected the response: 
 ascanbeta.cookieslack.desc = Repeated GET requests: drop a different cookie each time, followed by normal request with all cookies to stabilize session, compare responses against original baseline GET. This can reveal areas where cookie based authentication/attributes are not actually enforced.
 ascanbeta.cookieslack.endline = \n
 ascanbeta.cookieslack.name = Cookie Slack Detector

addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ExpressionLanguageInjectionScanRuleUnitTest.java

@@ -20,11 +20,16 @@
 package org.zaproxy.zap.extension.ascanrulesBeta;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.emptyString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 
+import java.util.List;
 import java.util.Map;
 import org.junit.jupiter.api.Test;
+import org.parosproxy.paros.core.scanner.Alert;
 import org.zaproxy.addon.commonlib.CommonAlertTag;
 
 class ExpressionLanguageInjectionScanRuleUnitTest
@@ -64,4 +69,18 @@ void shouldReturnExpectedMappings() {
                 tags.get(CommonAlertTag.WSTG_V42_INPV_11_CODE_INJ.getTag()),
                 is(equalTo(CommonAlertTag.WSTG_V42_INPV_11_CODE_INJ.getValue())));
     }
+
+    @Test
+    void shouldHaveExpectedExampleAlert() {
+        // Given /  When
+        List<Alert> alerts = rule.getExampleAlerts();
+        // Then
+        assertThat(alerts, hasSize(1));
+        Alert alert = alerts.get(0);
+        assertThat(alert.getRisk(), is(equalTo(rule.getRisk())));
+        assertThat(alert.getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
+        assertThat(alert.getParam(), is(equalTo("foo")));
+        assertThat(alert.getAttack(), is(not(emptyString())));
+        assertThat(alert.getEvidence(), is(not(emptyString())));
+    }
 }

addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/SlackerCookieScanRuleUnitTest.java

@@ -21,10 +21,13 @@
 
 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 java.util.Map;
 import org.junit.jupiter.api.Test;
+import org.parosproxy.paros.core.scanner.Alert;
 import org.zaproxy.addon.commonlib.CommonAlertTag;
 
 class SlackerCookieScanRuleUnitTest extends ActiveScannerTest<SlackerCookieScanRule> {
@@ -63,4 +66,25 @@ void shouldReturnExpectedMappings() {
                 tags.get(CommonAlertTag.WSTG_V42_SESS_02_COOKIE_ATTRS.getTag()),
                 is(equalTo(CommonAlertTag.WSTG_V42_SESS_02_COOKIE_ATTRS.getValue())));
     }
+
+    @Test
+    void shouldHaveExpectedExampleAlert() {
+        // Given /  When
+        List<Alert> alerts = rule.getExampleAlerts();
+        // Then
+        assertThat(alerts, hasSize(1));
+        Alert alert = alerts.get(0);
+        assertThat(alert.getRisk(), is(equalTo(Alert.RISK_INFO)));
+        assertThat(alert.getConfidence(), is(equalTo(Alert.CONFIDENCE_LOW)));
+        assertThat(
+                alert.getOtherInfo(),
+                is(
+                        equalTo(
+                                "Cookies that don't have "
+                                        + "expected effects can reveal flaws in application logic. In "
+                                        + "the worst case, this can reveal where authentication via cookie "
+                                        + "token(s) is not actually enforced.\n"
+                                        + "These cookies affected the response: oops\n"
+                                        + "These cookies did NOT affect the response: bar,foo\n")));
+    }
 }