pscanrules: More example alerts

kingthorin · kingthorin · commit b7ccd1b06579 · 2024-02-05T14:05:02.000-05:00
- CHANGELOGS &gt; Added change notes.
- Scan rules &gt; Added example alert functionality.
- UnitTests &gt; Added tests for example alerts.

Signed-off-by: kingthorin &lt;kingthorin@users.noreply.github.com&gt;
diff --git a/addOns/pscanrules/CHANGELOG.md b/addOns/pscanrules/CHANGELOG.md
@@ -10,10 +10,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
     - Timestamp Disclosure - Unix
     - Hash Disclosure
     - Cross-Domain Misconfiguration
+    - Weak Authentication Method
+    - Reverse Tabnabbing
+    - CSRF Countermeasures
+- The following scan rules now have alert references (Issue 7100):
+    - Weak Authentication Method
 - The references for Alerts from the following rules were also updated (Issue 8262):
     - Timestamp Disclosure - Unix 
     - Hash Disclosure
     - View State Scan Rule 
+    - Weak Authentication Method
 
 ## [55] - 2024-01-26
 ### Changed
diff --git a/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/CsrfCountermeasuresScanRule.java b/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/CsrfCountermeasuresScanRule.java
@@ -208,34 +208,25 @@ public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
 
                 int risk = Alert.RISK_MEDIUM;
                 String desc = Constant.messages.getString("pscanrules.noanticsrftokens.desc");
-                String extraInfo =
-                        Constant.messages.getString(
-                                "pscanrules.noanticsrftokens.alert.extrainfo",
-                                tokenNamesFlattened,
-                                formDetails);
+                String extraInfo = getExtraInfo(tokenNamesFlattened, formDetails);
                 if (hasSecurityAnnotation) {
                     risk = Alert.RISK_INFO;
                     extraInfo =
                             Constant.messages.getString(
                                     "pscanrules.noanticsrftokens.extrainfo.annotation");
                 }
 
-                newAlert()
-                        .setRisk(risk)
-                        .setConfidence(Alert.CONFIDENCE_LOW)
-                        .setDescription(desc + "\n" + getDescription())
-                        .setOtherInfo(extraInfo)
-                        .setSolution(getSolution())
-                        .setReference(getReference())
-                        .setEvidence(evidence)
-                        .setCweId(getCweId())
-                        .setWascId(getWascId())
-                        .raise();
+                buildAlert(risk, desc, extraInfo, evidence).raise();
             }
         }
         LOGGER.debug("\tScan of record {} took {} ms", id, System.currentTimeMillis() - start);
     }
 
+    private String getExtraInfo(String tokenNamesFlattened, String formDetails) {
+        return Constant.messages.getString(
+                "pscanrules.noanticsrftokens.alert.extrainfo", tokenNamesFlattened, formDetails);
+    }
+
     private boolean formOnIgnoreList(Element formElement, List<String> ignoreList) {
         String id = formElement.getAttributeValue("id");
         String name = formElement.getAttributeValue("name");
@@ -293,6 +284,31 @@ public int getWascId() {
         return 9;
     }
 
+    private AlertBuilder buildAlert(int risk, String desc, String extraInfo, String evidence) {
+        return newAlert()
+                .setRisk(risk)
+                .setConfidence(Alert.CONFIDENCE_LOW)
+                .setDescription(desc + "\n" + getDescription())
+                .setOtherInfo(extraInfo)
+                .setSolution(getSolution())
+                .setReference(getReference())
+                .setEvidence(evidence)
+                .setCweId(getCweId())
+                .setWascId(getWascId());
+    }
+
+    @Override
+    public List<Alert> getExampleAlerts() {
+        return List.of(
+                buildAlert(
+                                Alert.RISK_MEDIUM,
+                                Constant.messages.getString("pscanrules.noanticsrftokens.desc"),
+                                getExtraInfo(
+                                        "[token, csrfToken, csrf-token]", "[Form 1: \"name\" ]"),
+                                "<form name=\"someName\" data-no-csrf>")
+                        .build());
+    }
+
     protected ExtensionAntiCSRF getExtensionAntiCSRF() {
         if (extensionAntiCSRF == null) {
             return Control.getSingleton()
diff --git a/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/InsecureAuthenticationScanRule.java b/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/InsecureAuthenticationScanRule.java
@@ -183,27 +183,8 @@ public void scanHttpRequestSend(HttpMessage msg, int id) {
                         digestInfo = authValues[1]; // info to output in the logging message.
                     }
 
-                    newAlert()
-                            .setName(
-                                    Constant.messages.getString(
-                                            "pscanrules.authenticationcredentialscaptured.name"))
-                            .setRisk(alertRisk)
-                            .setConfidence(alertConf)
-                            .setDescription(
-                                    Constant.messages.getString(
-                                            "pscanrules.authenticationcredentialscaptured.desc"))
-                            .setOtherInfo(extraInfo)
-                            .setSolution(
-                                    Constant.messages.getString(
-                                            "pscanrules.authenticationcredentialscaptured.soln"))
-                            .setReference(
-                                    Constant.messages.getString(
-                                            "pscanrules.authenticationcredentialscaptured.refs"))
-                            .setCweId(287) // CWE Id - Improper authentication
-                            .setWascId(1) // WASC Id - Insufficient authentication
-                            .raise();
+                    buildCapturedAlert(alertRisk, alertConf, extraInfo).raise();
 
-                    // and log it, without internationalising it.
                     LOGGER.info(
                             "Authentication Credentials were captured. [{}] [{}] uses insecure authentication mechanism [{}], revealing username [{}] and password/additional information [{}]",
                             method,
@@ -269,18 +250,58 @@ public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
             for (String auth : authHeaders) {
                 if (auth.toLowerCase().indexOf("basic") > -1
                         || auth.toLowerCase().indexOf("digest") > -1) {
-                    newAlert()
-                            .setRisk(Alert.RISK_MEDIUM)
-                            .setConfidence(Alert.CONFIDENCE_MEDIUM)
-                            .setDescription(getDescription())
-                            .setSolution(getSolution())
-                            .setReference(getReference())
-                            .setEvidence(HttpHeader.WWW_AUTHENTICATE + ": " + auth)
-                            .setCweId(getCweId())
-                            .setWascId(getWascId())
-                            .raise();
+                    buildAlert(auth).raise();
                 }
             }
         }
     }
+
+    private AlertBuilder buildCapturedAlert(int alertRisk, int alertConf, String extraInfo) {
+        return newAlert()
+                .setName(
+                        Constant.messages.getString(
+                                "pscanrules.authenticationcredentialscaptured.name"))
+                .setRisk(alertRisk)
+                .setConfidence(alertConf)
+                .setDescription(
+                        Constant.messages.getString(
+                                "pscanrules.authenticationcredentialscaptured.desc"))
+                .setOtherInfo(extraInfo)
+                .setSolution(
+                        Constant.messages.getString(
+                                "pscanrules.authenticationcredentialscaptured.soln"))
+                .setReference(
+                        Constant.messages.getString(
+                                "pscanrules.authenticationcredentialscaptured.refs"))
+                .setCweId(287) // CWE Id - Improper authentication
+                .setWascId(1) // WASC Id - Insufficient authentication
+                .setAlertRef(getPluginId() + "-1");
+    }
+
+    private AlertBuilder buildAlert(String auth) {
+        return newAlert()
+                .setRisk(Alert.RISK_MEDIUM)
+                .setConfidence(Alert.CONFIDENCE_MEDIUM)
+                .setDescription(getDescription())
+                .setSolution(getSolution())
+                .setReference(getReference())
+                .setEvidence(HttpHeader.WWW_AUTHENTICATE + ": " + auth)
+                .setCweId(getCweId())
+                .setWascId(getWascId())
+                .setAlertRef(getPluginId() + "-2");
+    }
+
+    @Override
+    public List<Alert> getExampleAlerts() {
+        return List.of(
+                buildCapturedAlert(
+                                Alert.RISK_MEDIUM,
+                                Alert.CONFIDENCE_MEDIUM,
+                                "[POST] "
+                                        + "[http://www.example.com] uses insecure authentication mechanism [Digest], "
+                                        + "revealing username [admin] and additional information "
+                                        + "[username=\"admin\", realm=\"members only\"].")
+                        .build(),
+                buildAlert("Basic realm=\"Private\"").build());
+    }
 }
diff --git a/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/LinkTargetScanRule.java b/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/LinkTargetScanRule.java
@@ -130,20 +130,23 @@ private boolean checkElement(Element link) {
         if (relAtt != null) {
             relAtt = relAtt.toLowerCase();
             if (relAtt.contains(OPENER) && !relAtt.contains(NOOPENER)) {
-                newAlert()
-                        .setRisk(Alert.RISK_MEDIUM)
-                        .setConfidence(Alert.CONFIDENCE_MEDIUM)
-                        .setDescription(getDescription())
-                        .setSolution(getSolution())
-                        .setReference(getReference())
-                        .setEvidence(link.toString())
-                        .raise();
+                buildAlert(link.toString()).raise();
                 return true;
             }
         }
         return false;
     }
 
+    private AlertBuilder buildAlert(String evidence) {
+        return newAlert()
+                .setRisk(Alert.RISK_MEDIUM)
+                .setConfidence(Alert.CONFIDENCE_MEDIUM)
+                .setDescription(getDescription())
+                .setSolution(getSolution())
+                .setReference(getReference())
+                .setEvidence(evidence);
+    }
+
     @Override
     public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
         if (msg.getResponseBody().length() == 0 || !msg.getResponseHeader().isHtml()) {
@@ -190,4 +193,12 @@ private String getReference() {
     public Map<String, String> getAlertTags() {
         return ALERT_TAGS;
     }
+
+    @Override
+    public List<Alert> getExampleAlerts() {
+        return List.of(
+                buildAlert(
+                                "<a href=\"https://www.example3.com/page1\" rel=\"opener\" target=\"_blank\">link</a>")
+                        .build());
+    }
 }
diff --git a/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/ModernAppDetectionScanRule.java b/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/ModernAppDetectionScanRule.java
@@ -84,17 +84,20 @@ public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
 
         if (evidence != null && evidence.length() > 0) {
             // we found something
-            newAlert()
-                    .setRisk(Alert.RISK_INFO)
-                    .setConfidence(Alert.CONFIDENCE_MEDIUM)
-                    .setDescription(getDescription())
-                    .setOtherInfo(otherInfo)
-                    .setSolution(getSolution())
-                    .setEvidence(evidence)
-                    .raise();
+            buildAlert(otherInfo, evidence).raise();
         }
     }
 
+    private AlertBuilder buildAlert(String otherInfo, String evidence) {
+        return newAlert()
+                .setRisk(Alert.RISK_INFO)
+                .setConfidence(Alert.CONFIDENCE_MEDIUM)
+                .setDescription(getDescription())
+                .setOtherInfo(otherInfo)
+                .setSolution(getSolution())
+                .setEvidence(evidence);
+    }
+
     @Override
     public int getPluginId() {
         return 10109;
@@ -107,4 +110,13 @@ private String getDescription() {
     private String getSolution() {
         return Constant.messages.getString(MESSAGE_PREFIX + "soln");
     }
+
+    @Override
+    public List<Alert> getExampleAlerts() {
+        return List.of(
+                buildAlert(
+                                Constant.messages.getString(MESSAGE_PREFIX + "other.self"),
+                                "<a href=\"link\" target=\"_self\">Link</a>")
+                        .build());
+    }
 }
diff --git a/addOns/pscanrules/src/main/resources/org/zaproxy/zap/extension/pscanrules/resources/Messages.properties b/addOns/pscanrules/src/main/resources/org/zaproxy/zap/extension/pscanrules/resources/Messages.properties
@@ -25,7 +25,7 @@ pscanrules.authenticationcredentialscaptured.alert.basicauth.extrainfo = [{0}] [
 pscanrules.authenticationcredentialscaptured.alert.digestauth.extrainfo = [{0}] [{1}] uses insecure authentication mechanism [{2}], revealing username [{3}] and additional information [{4}].
 pscanrules.authenticationcredentialscaptured.desc = An insecure authentication mechanism is in use. This allows an attacker on the network access to the userid and password of the authenticated user. For Basic Authentication, the attacker must merely monitor the network traffic until a Basic Authentication request is received, and then base64 decode the username and password.  For Digest Authentication, the attacker has access to the username, and possibly also the password, if the hash (including a nonce) can be successfully cracked, or if a Man-In-The-Middle attack is mounted.\nThe attacker eavesdrops on the network until an authentication has completed.
 pscanrules.authenticationcredentialscaptured.name = Authentication Credentials Captured
-pscanrules.authenticationcredentialscaptured.refs = https://owasp.org/www-community/attacks/Brute_force_attack\nhttp://en.wikipedia.org/wiki/Digest_access_authentication
+pscanrules.authenticationcredentialscaptured.refs = https://owasp.org/www-community/attacks/Brute_force_attack\nhttps://en.wikipedia.org/wiki/Digest_access_authentication
 pscanrules.authenticationcredentialscaptured.soln = Use HTTPS, and use a secure authentication mechanism that does not transmit the userid or password in an un-encrypted fashion.  In particular, avoid use of the Basic Authentication mechanism, since this trivial obfuscation mechanism is easily broken. 
 
 pscanrules.bigredirects.desc = The server has responded with a redirect that seems to provide a large response. This may indicate that although the server sent a redirect it also responded with body content (which may include sensitive details, PII, etc.).
diff --git a/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/CsrfCountermeasuresScanRuleUnitTest.java b/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/CsrfCountermeasuresScanRuleUnitTest.java
@@ -123,6 +123,20 @@ void shouldReturnExpectedMappings() {
                 is(equalTo(CommonAlertTag.WSTG_V42_SESS_05_CSRF.getValue())));
     }
 
+    @Test
+    void shouldHaveExpectedExampleAlerts() {
+        // Given / When
+        List<Alert> alerts = rule.getExampleAlerts();
+        // Then
+        assertThat(alerts.size(), is(equalTo(1)));
+    }
+
+    @Test
+    @Override
+    public void shouldHaveValidReferences() {
+        super.shouldHaveValidReferences();
+    }
+
     @Test
     void shouldNotRaiseAlertIfContentTypeIsNotHTML() {
         // Given
diff --git a/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/InsecureAuthenticationScanRuleUnitTest.java b/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/InsecureAuthenticationScanRuleUnitTest.java
@@ -25,6 +25,7 @@
 
 import java.io.IOException;
 import java.util.Base64;
+import java.util.List;
 import java.util.Map;
 import org.apache.commons.httpclient.URI;
 import org.junit.jupiter.api.Test;
@@ -97,6 +98,24 @@ void shouldReturnExpectedMappings() {
                 is(equalTo(CommonAlertTag.WSTG_V42_ATHN_01_CREDS_NO_CRYPTO.getValue())));
     }
 
+    @Test
+    void shouldHaveExpectedExampleAlerts() {
+        // Given / When
+        List<Alert> alerts = rule.getExampleAlerts();
+        // Then
+        assertThat(alerts.size(), is(equalTo(2)));
+        Alert capturedAlert = alerts.get(0);
+        assertThat(capturedAlert.getAlertRef(), is(equalTo("10105-1")));
+        Alert weakAlert = alerts.get(1);
+        assertThat(weakAlert.getAlertRef(), is(equalTo("10105-2")));
+    }
+
+    @Test
+    @Override
+    public void shouldHaveValidReferences() {
+        super.shouldHaveValidReferences();
+    }
+
     @Test
     void shouldBeSecureIfHttpUsedWithSsl() throws HttpMalformedHeaderException {
         // Given
diff --git a/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/LinkTargetScanRuleUnitTest.java b/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/LinkTargetScanRuleUnitTest.java
@@ -28,11 +28,13 @@
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mock;
 import org.mockito.Mock.Strictness;
+import org.parosproxy.paros.core.scanner.Alert;
 import org.parosproxy.paros.core.scanner.Plugin.AlertThreshold;
 import org.parosproxy.paros.model.Model;
 import org.parosproxy.paros.model.Session;
@@ -492,4 +494,18 @@ void shouldReturnExpectedMappings() {
                 tags.get(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getTag()),
                 is(equalTo(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getValue())));
     }
+
+    @Test
+    void shouldHaveExpectedExampleAlerts() {
+        // Given / When
+        List<Alert> alerts = rule.getExampleAlerts();
+        // Then
+        assertThat(alerts.size(), is(equalTo(1)));
+    }
+
+    @Test
+    @Override
+    public void shouldHaveValidReferences() {
+        super.shouldHaveValidReferences();
+    }
 }
diff --git a/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/ModernAppDetectionScanRuleUnitTest.java b/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/ModernAppDetectionScanRuleUnitTest.java
diff --git a/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/UserControlledJavascriptEventScanRuleUnitTest.java b/addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/UserControlledJavascriptEventScanRuleUnitTest.java
