Merge pull request #6007 from kingthorin/client-fix

client: SAST (SonarLint) Fixes and update CWEs

addOns/client/CHANGELOG.md

@@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ## Unreleased
 ### Changed
 - Update minimum ZAP version to 2.16.0.
+- Maintenance changes.
+- The current passive scan rules now uses a more specific CWE (Issue 8712).
 
 ### Added
 - Added support for Browser Based Authentication when installed in conjunction with the Auth Helper add-on.

addOns/client/src/main/java/org/zaproxy/addon/client/ClientOptions.java

@@ -21,7 +21,6 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.stream.Collectors;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.zaproxy.addon.commonlib.Constants;
@@ -86,7 +85,7 @@ protected void parseImpl() {
                     getConfig().getList(PSCAN_DISABLED_RULES_KEY).stream()
                             .map(Object::toString)
                             .map(Integer::parseInt)
-                            .collect(Collectors.toList());
+                            .toList();
         } catch (Exception e) {
             LOGGER.warn(e.getMessage(), e);
             pscanRulesDisabled = new ArrayList<>();

addOns/client/src/main/java/org/zaproxy/addon/client/ExtensionClientIntegration.java

@@ -19,6 +19,7 @@
  */
 package org.zaproxy.addon.client;
 
+import java.awt.event.InputEvent;
 import java.awt.event.KeyEvent;
 import java.io.File;
 import java.io.IOException;
@@ -318,25 +319,17 @@ public void postInit() {
             LOGGER.error(e.getMessage(), e);
         }
         eventConsumer =
-                new EventConsumer() {
-
-                    @Override
-                    public void eventReceived(Event event) {
-                        // Listen for AJAX Spider events
-                        if (ScanEventPublisher.SCAN_STARTED_EVENT.equals(event.getEventType())) {
-                            // Record this for when we get the stopped event
-                            lastAjaxSpiderStartEvent = event;
-                        } else if (ScanEventPublisher.SCAN_STOPPED_EVENT.equals(
-                                event.getEventType())) {
-                            // See if we can find any missed URLs in the DOM
-                            MissingUrlsThread mut =
-                                    new MissingUrlsThread(
-                                            getModel(),
-                                            lastAjaxSpiderStartEvent,
-                                            clientTree.getRoot());
-                            lastAjaxSpiderStartEvent = null;
-                            mut.start();
-                        }
+                event -> {
+                    if (ScanEventPublisher.SCAN_STARTED_EVENT.equals(event.getEventType())) {
+                        // Record this for when we get the stopped event
+                        lastAjaxSpiderStartEvent = event;
+                    } else if (ScanEventPublisher.SCAN_STOPPED_EVENT.equals(event.getEventType())) {
+                        // See if we can find any missed URLs in the DOM
+                        MissingUrlsThread mut =
+                                new MissingUrlsThread(
+                                        getModel(), lastAjaxSpiderStartEvent, clientTree.getRoot());
+                        lastAjaxSpiderStartEvent = null;
+                        mut.start();
                     }
                 };
 
@@ -669,7 +662,7 @@ private ZapMenuItem getMenuItemCustomScan() {
                             "client.spider.menu.tools.label",
                             getView()
                                     .getMenuShortcutKeyStroke(
-                                            KeyEvent.VK_C, KeyEvent.ALT_DOWN_MASK, false));
+                                            KeyEvent.VK_C, InputEvent.ALT_DOWN_MASK, false));
             menuItemCustomScan.setEnabled(Control.getSingleton().getMode() != Mode.safe);
             menuItemCustomScan.setIcon(getIcon());
 

addOns/client/src/main/java/org/zaproxy/addon/client/MissingUrlsThread.java

@@ -63,7 +63,7 @@ public void run() {
         traverseMap(rootNode);
     }
 
-    private void persistToHistoryAndSitesTree(HttpMessage msg) {
+    private static void persistToHistoryAndSitesTree(HttpMessage msg) {
         HistoryReference historyRef;
         ExtensionHistory extHistory =
                 Control.getSingleton().getExtensionLoader().getExtension(ExtensionHistory.class);

addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientSideComponent.java

@@ -100,9 +100,7 @@ public String getType() {
 
     public boolean isStorageEvent() {
         switch (type) {
-            case "Cookies":
-            case "localStorage":
-            case "sessionStorage":
+            case "Cookies", "localStorage", "sessionStorage":
                 return true;
             default:
                 return false;

addOns/client/src/main/java/org/zaproxy/addon/client/pscan/ClientPassiveScanController.java

@@ -23,7 +23,6 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.function.Predicate;
-import java.util.stream.Collectors;
 
 public class ClientPassiveScanController {
 
@@ -54,15 +53,11 @@ public List<ClientPassiveScanRule> getEnabledScanRules() {
         if (!enabled) {
             return Collections.emptyList();
         }
-        return scanRules.stream()
-                .filter(ClientPassiveScanRule::isEnabled)
-                .collect(Collectors.toList());
+        return scanRules.stream().filter(ClientPassiveScanRule::isEnabled).toList();
     }
 
     public List<ClientPassiveScanRule> getDisabledScanRules() {
-        return scanRules.stream()
-                .filter(Predicate.not(ClientPassiveScanRule::isEnabled))
-                .collect(Collectors.toList());
+        return scanRules.stream().filter(Predicate.not(ClientPassiveScanRule::isEnabled)).toList();
     }
 
     /**

addOns/client/src/main/java/org/zaproxy/addon/client/pscan/ClientPassiveScanHelper.java

@@ -108,7 +108,7 @@ public static String base64Decode(String value) {
     public void raiseAlert(Alert alert, HistoryReference hr) {
         if (hr == null) {
             LOGGER.warn(
-                    "Failed to find history reference for URL {}, unable to raise alert",
+                    "Failed to find history reference for URL {}, unable to raise alert {}",
                     alert.getUri(),
                     alert.toPluginXML(ClientUtils.stripUrlFragment(alert.getUri())));
         } else {

addOns/client/src/main/java/org/zaproxy/addon/client/pscan/InformationInStorageScanRule.java

@@ -74,7 +74,8 @@ private Alert.Builder getAlertBuilder(ReportedObject obj) {
                                         obj.getId() + "=" + value,
                                         obj.getId() + "=" + decodedValue))
                 .setSolution(Constant.messages.getString("client.pscan.infoinstorage.solution"))
-                .setCweId(200) // CWE Id: 200 - Information Exposure
+                .setCweId(359) //  CWE-359: Exposure of Private Personal Information to an
+                // Unauthorized Actor
                 .setWascId(13); // WASC Id: 13 - Information Leakage
     }
 

addOns/client/src/main/java/org/zaproxy/addon/client/pscan/JwtInStorageScanRule.java

@@ -110,7 +110,7 @@ private Alert.Builder getAlertBuilder(
                                 "client.pscan.jwtinstorage.solution."
                                         + (isLocal ? "local" : "session")))
                 .setReference("https://www.zaproxy.org/blog/2020-09-03-zap-jwt-scanner/")
-                .setCweId(200) // CWE Id: 200 - Information Exposure
+                .setCweId(922) //  CWE-922: Insecure Storage of Sensitive Information
                 .setWascId(13); // WASC Id: 13 - Information Leakage
     }
 

addOns/client/src/main/java/org/zaproxy/addon/client/pscan/OptionsPassiveScan.java

@@ -25,7 +25,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Predicate;
-import java.util.stream.Collectors;
 import javax.swing.BorderFactory;
 import javax.swing.JCheckBox;
 import javax.swing.JLabel;
@@ -172,7 +171,7 @@ public List<Integer> getDisabledScannerIds() {
             return this.scanController.getAllScanRules().stream()
                     .filter(Predicate.not(ClientPassiveScanRule::isEnabled))
                     .map(ClientPassiveScanRule::getId)
-                    .collect(Collectors.toList());
+                    .toList();
         }
 
         public void setDisabledScannerIds(List<Integer> disabledScanIds) {
@@ -183,7 +182,6 @@ public void setDisabledScannerIds(List<Integer> disabledScanIds) {
                                 if (disabledScanIds.stream().noneMatch(i -> (i == s.getId()))) {
                                     enabledPscanList.add(s);
                                 }
-                                ;
                             });
             this.fireTableDataChanged();
         }

addOns/client/src/main/java/org/zaproxy/addon/client/pscan/SensitiveInfoInStorageScanRule.java

@@ -37,10 +37,20 @@ public class SensitiveInfoInStorageScanRule extends ClientPassiveAbstractScanRul
     private static final String SESSION_STORAGE = "sessionStorage";
 
     private enum InfoType {
-        cc,
-        email,
-        ssn
-    };
+        CC("cc"),
+        EMAIL("email"),
+        SSN("ssn");
+
+        private String id;
+
+        InfoType(String id) {
+            this.id = id;
+        }
+
+        String getId() {
+            return this.id;
+        }
+    }
 
     // Patterns copied from {@link InformationDisclosureInUrlScanRule}
     static Pattern emailAddressPattern =
@@ -49,8 +59,8 @@ private enum InfoType {
     // https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s20.html
     static Pattern creditCardPattern =
             Pattern.compile(
-                    "\\b(?:4[0-9]{12}(?:[0-9]{3})?|5[1-5][0-9]{14}|6(?:011|5[0-9][0-9])[0-9]{12}|3[47][0-9]{13}|3(?:0[0-5]|[68][0-9])[0-9]{11}|(?:2131|1800|35\\d{3})\\d{11})\\b");
-    static Pattern usSSNPattern = Pattern.compile("\\b[0-9]{3}-[0-9]{2}-[0-9]{4}\\b");
+                    "\\b(?:4\\d{12}(?:\\d{3})?|5[1-5]\\d{14}|6(?:011|5\\d\\d)\\d{12}|3[47]\\d{13}|3(?:0[0-5]|[68]\\d)\\d{11}|(?:2131|1800|35\\d{3})\\d{11})\\b");
+    static Pattern usSSNPattern = Pattern.compile("\\b\\d{3}-\\d{2}-\\d{4}\\b");
 
     @Override
     public String getName() {
@@ -70,15 +80,15 @@ public void scanReportedObject(ReportedObject obj, ClientPassiveScanHelper helpe
             String decodedValue = ClientPassiveScanHelper.base64Decode(value);
 
             if (isCreditCard(value) || isCreditCard(decodedValue)) {
-                helper.raiseAlert(this.getAlertBuilder(obj, decodedValue, InfoType.cc).build(), hr);
+                helper.raiseAlert(this.getAlertBuilder(obj, decodedValue, InfoType.CC).build(), hr);
             }
             if (isEmailAddress(value) || isEmailAddress(decodedValue)) {
                 helper.raiseAlert(
-                        this.getAlertBuilder(obj, decodedValue, InfoType.email).build(), hr);
+                        this.getAlertBuilder(obj, decodedValue, InfoType.EMAIL).build(), hr);
             }
             if (isUsSSN(value) || isUsSSN(decodedValue)) {
                 helper.raiseAlert(
-                        this.getAlertBuilder(obj, decodedValue, InfoType.ssn).build(), hr);
+                        this.getAlertBuilder(obj, decodedValue, InfoType.SSN).build(), hr);
             }
         }
     }
@@ -99,14 +109,17 @@ protected Alert.Builder getAlertBuilder(
                 .setOtherInfo(
                         decodedValue == null
                                 ? Constant.messages.getString(
-                                        "client.pscan.seninfoinstorage.other." + infoType,
+                                        "client.pscan.seninfoinstorage.other." + infoType.getId(),
                                         obj.getId() + "=" + obj.getText())
                                 : Constant.messages.getString(
-                                        "client.pscan.seninfoinstorage.other.base64." + infoType,
+                                        "client.pscan.seninfoinstorage.other.base64."
+                                                + infoType.getId(),
                                         obj.getId() + "=" + obj.getText(),
                                         obj.getId() + "=" + decodedValue))
                 .setSolution(Constant.messages.getString("client.pscan.seninfoinstorage.solution"))
-                .setCweId(200) // CWE Id: 200 - Information Exposure
+                .setCweId(
+                        359) // CWE-359: Exposure of Private Personal Information to an Unauthorized
+                // Actor
                 .setWascId(13); // WASC Id: 13 - Information Leakage
     }
 
@@ -119,29 +132,29 @@ public List<Alert> getExampleAlerts() {
         obj.put("tagname", "");
         obj.put("id", "key");
         obj.put("text", "value");
-        alerts.add(getAlertBuilder(new ReportedElement(obj), null, InfoType.cc).build());
+        alerts.add(getAlertBuilder(new ReportedElement(obj), null, InfoType.CC).build());
         obj.put("type", SESSION_STORAGE);
-        alerts.add(getAlertBuilder(new ReportedElement(obj), null, InfoType.email).build());
+        alerts.add(getAlertBuilder(new ReportedElement(obj), null, InfoType.EMAIL).build());
         return alerts;
     }
 
-    private boolean isEmailAddress(String value) {
+    private static boolean isEmailAddress(String value) {
         if (value == null) {
             return false;
         }
         Matcher matcher = emailAddressPattern.matcher(value);
         return matcher.find();
     }
 
-    private boolean isCreditCard(String value) {
+    private static boolean isCreditCard(String value) {
         if (value == null) {
             return false;
         }
         Matcher matcher = creditCardPattern.matcher(value);
         return matcher.find();
     }
 
-    private boolean isUsSSN(String value) {
+    private static boolean isUsSSN(String value) {
         if (value == null) {
             return false;
         }

addOns/client/src/main/java/org/zaproxy/addon/client/spider/ClientSpider.java

@@ -158,7 +158,7 @@ private List<String> getUnvisitedUrls() {
     private void getUnvisitedUrls(ClientNode node, List<String> urls) {
         String nodeUrl = node.getUserObject().getUrl();
         if (nodeUrl.startsWith(targetUrl)
-                && !(nodeUrl.length() == targetUrl.length())
+                && nodeUrl.length() != targetUrl.length()
                 && !node.isStorage()
                 && !node.getUserObject().isVisited()) {
             urls.add(nodeUrl);
@@ -258,7 +258,7 @@ public void eventReceived(Event event) {
     }
 
     public int getProgress() {
-        if (finished & !stopped) {
+        if (finished && !stopped) {
             return 100;
         } else if (this.tasksTotalCount <= 1) {
             // Still waiting for the first request to be processed
@@ -282,7 +282,7 @@ public void pause() {
     }
 
     public void resume() {
-        this.pausedTasks.forEach(ct -> executeTask(ct));
+        this.pausedTasks.forEach(this::executeTask);
         this.paused = false;
     }
 

addOns/client/src/main/java/org/zaproxy/addon/client/spider/ClientSpiderDialog.java

@@ -143,7 +143,7 @@ private void init(SiteNode node, String url) {
         }
 
         this.addCheckBoxField(0, FIELD_SUBTREE_ONLY, subtreeOnlyPreviousCheckedState);
-        this.addComboField(0, FIELD_BROWSER, new ArrayList<String>(), null);
+        this.addComboField(0, FIELD_BROWSER, new ArrayList<>(), null);
 
         // This option is always read from the 'global' options
         this.addCheckBoxField(0, FIELD_ADVANCED, params.isShowAdvancedDialog());
@@ -235,16 +235,13 @@ public void addUrlSelectField(
         selectButton.setIcon(
                 new ImageIcon(View.class.getResource("/resource/icon/16/094.png"))); // Globe icon
         selectButton.addActionListener(
-                new java.awt.event.ActionListener() {
-                    @Override
-                    public void actionPerformed(java.awt.event.ActionEvent e) {
-                        NodeSelectDialog nsd = new NodeSelectDialog(ClientSpiderDialog.this);
-                        nsd.setAllowRoot(allowRoot);
-                        SiteNode node = nsd.showDialog((SiteNode) null);
-                        if (node != null) {
-                            urlStartField.setText(getNodeText(node));
-                            siteNodeSelected(fieldLabel, node);
-                        }
+                e -> {
+                    NodeSelectDialog nsd = new NodeSelectDialog(ClientSpiderDialog.this);
+                    nsd.setAllowRoot(allowRoot);
+                    SiteNode node = nsd.showDialog((SiteNode) null);
+                    if (node != null) {
+                        urlStartField.setText(getNodeText(node));
+                        siteNodeSelected(fieldLabel, node);
                     }
                 });
         JPanel panel = new JPanel();
@@ -351,27 +348,27 @@ private String getStartUrl() {
     /** Use the save method to launch a scan */
     @Override
     public void save() {
-        ClientOptions params = this.extension.getClientParam();
+        ClientOptions clientParams = this.extension.getClientParam();
 
         String selectedBrowser = getSelectedBrowser();
         if (selectedBrowser != null) {
-            params.setBrowserId(selectedBrowser);
+            clientParams.setBrowserId(selectedBrowser);
         }
 
-        if (this.getBoolValue(FIELD_ADVANCED)) {
-            params.setThreadCount(this.getIntValue(FIELD_NUM_BROWSERS));
-            params.setMaxDepth(this.getIntValue(FIELD_DEPTH));
-            params.setMaxChildren(this.getIntValue(FIELD_CHILDREN));
-            params.setInitialLoadTimeInSecs(this.getIntValue(FIELD_INITIAL_LOAD_TIME));
-            params.setPageLoadTimeInSecs(this.getIntValue(FIELD_PAGE_LOAD_TIME));
-            params.setShutdownTimeInSecs(this.getIntValue(FIELD_SHUTDOWN_TIME));
-            params.setMaxDuration(this.getIntValue(FIELD_DURATION));
+        if (Boolean.TRUE.equals(this.getBoolValue(FIELD_ADVANCED))) {
+            clientParams.setThreadCount(this.getIntValue(FIELD_NUM_BROWSERS));
+            clientParams.setMaxDepth(this.getIntValue(FIELD_DEPTH));
+            clientParams.setMaxChildren(this.getIntValue(FIELD_CHILDREN));
+            clientParams.setInitialLoadTimeInSecs(this.getIntValue(FIELD_INITIAL_LOAD_TIME));
+            clientParams.setPageLoadTimeInSecs(this.getIntValue(FIELD_PAGE_LOAD_TIME));
+            clientParams.setShutdownTimeInSecs(this.getIntValue(FIELD_SHUTDOWN_TIME));
+            clientParams.setMaxDuration(this.getIntValue(FIELD_DURATION));
         }
 
         subtreeOnlyPreviousCheckedState = getBoolValue(FIELD_SUBTREE_ONLY);
 
         User user = this.getSelectedUser();
-        this.extension.runSpider(getStartUrl(), params, user);
+        this.extension.runSpider(getStartUrl(), clientParams, user);
     }
 
     /**