From ce2791845749d6455114e051edeea030e352a7ec Mon Sep 17 00:00:00 2001 From: kingthorin Date: Mon, 30 Jun 2025 11:05:08 -0400 Subject: [PATCH] ascanrules: CMDi split timing tests to new scan rule Signed-off-by: kingthorin --- addOns/ascanrules/CHANGELOG.md | 1 + .../ascanrules/CommandInjectionScanRule.java | 299 +--------------- .../CommandInjectionTimingScanRule.java | 329 ++++++++++++++++++ .../resources/help/contents/ascanrules.html | 24 +- .../ascanrules/resources/Messages.properties | 6 +- .../ascanrules/CommandInjectionRuleTest.java | 85 +++++ .../CommandInjectionScanRuleUnitTest.java | 213 +----------- ...ommandInjectionTimingScanRuleUnitTest.java | 278 +++++++++++++++ 8 files changed, 733 insertions(+), 502 deletions(-) create mode 100644 addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionTimingScanRule.java create mode 100644 addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionRuleTest.java create mode 100644 addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionTimingScanRuleUnitTest.java diff --git a/addOns/ascanrules/CHANGELOG.md b/addOns/ascanrules/CHANGELOG.md index ec161d1a0bf..f67ed0a7b5b 100644 --- a/addOns/ascanrules/CHANGELOG.md +++ b/addOns/ascanrules/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - SQL Injection - Hypersonic - SQL Injection - SQLite - SQL Injection - PostgreSQL +- The Remote OS Command Injection scan rule has been broken into two rules; one feedback based, and one time based (Issue 7341). This includes assigning the time based rule ID 90037. ### Added - Rules (as applicable) have been tagged in relation to HIPAA and PCI DSS. diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java index 310938941e9..adb699b530a 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java @@ -27,14 +27,11 @@ import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.concurrent.ThreadLocalRandom; -import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.apache.commons.configuration.ConversionException; import org.apache.commons.text.StringEscapeUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -45,10 +42,8 @@ import org.parosproxy.paros.network.HttpMessage; import org.zaproxy.addon.commonlib.CommonAlertTag; import org.zaproxy.addon.commonlib.PolicyTag; -import org.zaproxy.addon.commonlib.timing.TimingUtils; import org.zaproxy.addon.commonlib.vulnerabilities.Vulnerabilities; import org.zaproxy.addon.commonlib.vulnerabilities.Vulnerability; -import org.zaproxy.zap.extension.ruleconfig.RuleConfigParam; import org.zaproxy.zap.model.Tech; import org.zaproxy.zap.model.TechSet; @@ -61,11 +56,8 @@ public class CommandInjectionScanRule extends AbstractAppParamPlugin implements CommonActiveScanRuleInfo { - /** The name of the rule to obtain the time, in seconds, for time-based attacks. */ - private static final String RULE_SLEEP_TIME = RuleConfigParam.RULE_COMMON_SLEEP_TIME; - /** Prefix for internationalised messages used by this rule */ - private static final String MESSAGE_PREFIX = "ascanrules.commandinjection."; + static final String MESSAGE_PREFIX = "ascanrules.commandinjection."; // *NIX OS Command constants private static final String NIX_TEST_CMD = "cat /etc/passwd"; @@ -90,7 +82,7 @@ public class CommandInjectionScanRule extends AbstractAppParamPlugin private static final Map WIN_OS_PAYLOADS = new LinkedHashMap<>(); private static final Map PS_PAYLOADS = new LinkedHashMap<>(); - private static final Map ALERT_TAGS; + static final Map ALERT_TAGS; static { Map alertTags = @@ -100,8 +92,7 @@ public class CommandInjectionScanRule extends AbstractAppParamPlugin CommonAlertTag.OWASP_2017_A01_INJECTION, CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ, CommonAlertTag.HIPAA, - CommonAlertTag.PCI_DSS, - CommonAlertTag.TEST_TIMING)); + CommonAlertTag.PCI_DSS)); alertTags.put(PolicyTag.API.getTag(), ""); alertTags.put(PolicyTag.DEV_CICD.getTag(), ""); alertTags.put(PolicyTag.DEV_STD.getTag(), ""); @@ -229,103 +220,12 @@ public class CommandInjectionScanRule extends AbstractAppParamPlugin NIX_OS_PAYLOADS.put("&&" + insertedCMD + NULL_BYTE_CHARACTER, NIX_CTRL_PATTERN); } - /** The default number of seconds used in time-based attacks (i.e. sleep commands). */ - private static final int DEFAULT_TIME_SLEEP_SEC = 5; - - // limit the maximum number of requests sent for time-based attack detection - private static final int BLIND_REQUESTS_LIMIT = 4; - - // error range allowable for statistical time-based blind attacks (0-1.0) - private static final double TIME_CORRELATION_ERROR_RANGE = 0.15; - private static final double TIME_SLOPE_ERROR_RANGE = 0.30; - - // *NIX Blind OS Command constants - private static final String NIX_BLIND_TEST_CMD = "sleep {0}"; - // Windows Blind OS Command constants - private static final String WIN_BLIND_TEST_CMD = "timeout /T {0}"; - // PowerSHell Blind Command constants - private static final String PS_BLIND_TEST_CMD = "start-sleep -s {0}"; - - // OS Command payloads for blind command Injection testing - private static final List NIX_BLIND_OS_PAYLOADS = new LinkedList<>(); - private static final List WIN_BLIND_OS_PAYLOADS = new LinkedList<>(); - private static final List PS_BLIND_PAYLOADS = new LinkedList<>(); - - static { - // No quote payloads - NIX_BLIND_OS_PAYLOADS.add("&" + NIX_BLIND_TEST_CMD + "&"); - NIX_BLIND_OS_PAYLOADS.add(";" + NIX_BLIND_TEST_CMD + ";"); - WIN_BLIND_OS_PAYLOADS.add("&" + WIN_BLIND_TEST_CMD); - WIN_BLIND_OS_PAYLOADS.add("|" + WIN_BLIND_TEST_CMD); - PS_BLIND_PAYLOADS.add(";" + PS_BLIND_TEST_CMD); - - // Double quote payloads - NIX_BLIND_OS_PAYLOADS.add("\"&" + NIX_BLIND_TEST_CMD + "&\""); - NIX_BLIND_OS_PAYLOADS.add("\";" + NIX_BLIND_TEST_CMD + ";\""); - WIN_BLIND_OS_PAYLOADS.add("\"&" + WIN_BLIND_TEST_CMD + "&\""); - WIN_BLIND_OS_PAYLOADS.add("\"|" + WIN_BLIND_TEST_CMD); - PS_BLIND_PAYLOADS.add("\";" + PS_BLIND_TEST_CMD); - - // Single quote payloads - NIX_BLIND_OS_PAYLOADS.add("'&" + NIX_BLIND_TEST_CMD + "&'"); - NIX_BLIND_OS_PAYLOADS.add("';" + NIX_BLIND_TEST_CMD + ";'"); - WIN_BLIND_OS_PAYLOADS.add("'&" + WIN_BLIND_TEST_CMD + "&'"); - WIN_BLIND_OS_PAYLOADS.add("'|" + WIN_BLIND_TEST_CMD); - PS_BLIND_PAYLOADS.add("';" + PS_BLIND_TEST_CMD); - - // Special payloads - NIX_BLIND_OS_PAYLOADS.add("\n" + NIX_BLIND_TEST_CMD + "\n"); // force enter - NIX_BLIND_OS_PAYLOADS.add("`" + NIX_BLIND_TEST_CMD + "`"); // backtick execution - NIX_BLIND_OS_PAYLOADS.add("||" + NIX_BLIND_TEST_CMD); // or control concatenation - NIX_BLIND_OS_PAYLOADS.add("&&" + NIX_BLIND_TEST_CMD); // and control concatenation - NIX_BLIND_OS_PAYLOADS.add("|" + NIX_BLIND_TEST_CMD + "#"); // pipe & comment - // FoxPro for running os commands - WIN_BLIND_OS_PAYLOADS.add("run " + WIN_BLIND_TEST_CMD); - PS_BLIND_PAYLOADS.add(";" + PS_BLIND_TEST_CMD + " #"); // chain & comment - - // uninitialized variable waf bypass - String insertedCMD = insertUninitVar(NIX_BLIND_TEST_CMD); - // No quote payloads - NIX_BLIND_OS_PAYLOADS.add("&" + insertedCMD + "&"); - NIX_BLIND_OS_PAYLOADS.add(";" + insertedCMD + ";"); - // Double quote payloads - NIX_BLIND_OS_PAYLOADS.add("\"&" + insertedCMD + "&\""); - NIX_BLIND_OS_PAYLOADS.add("\";" + insertedCMD + ";\""); - // Single quote payloads - NIX_BLIND_OS_PAYLOADS.add("'&" + insertedCMD + "&'"); - NIX_BLIND_OS_PAYLOADS.add("';" + insertedCMD + ";'"); - // Special payloads - NIX_BLIND_OS_PAYLOADS.add("\n" + insertedCMD + "\n"); - NIX_BLIND_OS_PAYLOADS.add("`" + insertedCMD + "`"); - NIX_BLIND_OS_PAYLOADS.add("||" + insertedCMD); - NIX_BLIND_OS_PAYLOADS.add("&&" + insertedCMD); - NIX_BLIND_OS_PAYLOADS.add("|" + insertedCMD + "#"); - } - // Logger instance private static final Logger LOGGER = LogManager.getLogger(CommandInjectionScanRule.class); // Get WASC Vulnerability description private static final Vulnerability VULN = Vulnerabilities.getDefault().get("wasc_31"); - /** The number of seconds used in time-based attacks (i.e. sleep commands). */ - private int timeSleepSeconds = DEFAULT_TIME_SLEEP_SEC; - - private enum TestType { - FEEDBACK("feedback-based"), - TIME("time-based"); - - private final String nameKey; - - private TestType(String nameKey) { - this.nameKey = nameKey; - } - - String getNameKey() { - return nameKey; - } - } - @Override public int getId() { return 90020; @@ -371,16 +271,6 @@ public Map getAlertTags() { return ALERT_TAGS; } - private Map getNeededAlertTags(TestType type) { - if (TestType.FEEDBACK.equals(type)) { - Map alertTags = new HashMap<>(); - alertTags.putAll(getAlertTags()); - alertTags.remove(CommonAlertTag.TEST_TIMING.getTag()); - return alertTags; - } - return getAlertTags(); - } - @Override public int getCweId() { return 78; @@ -396,35 +286,6 @@ public int getRisk() { return Alert.RISK_HIGH; } - private static String getOtherInfo(TestType testType, String testValue) { - return Constant.messages.getString( - MESSAGE_PREFIX + "otherinfo." + testType.getNameKey(), testValue); - } - - @Override - public void init() { - try { - timeSleepSeconds = this.getConfig().getInt(RULE_SLEEP_TIME, DEFAULT_TIME_SLEEP_SEC); - } catch (ConversionException e) { - LOGGER.debug( - "Invalid value for '{}': {}", - RULE_SLEEP_TIME, - this.getConfig().getString(RULE_SLEEP_TIME)); - } - LOGGER.debug("Sleep set to {} seconds", timeSleepSeconds); - } - - /** - * Gets the number of seconds used in time-based attacks. - * - *

Note: Method provided only to ease the unit tests. - * - * @return the number of seconds used in time-based attacks. - */ - int getTimeSleep() { - return timeSleepSeconds; - } - /** * Scan for OS Command Injection Vulnerabilities * @@ -444,22 +305,18 @@ public void scan(HttpMessage msg, String paramName, String value) { // Number of targets to try int targetCount = 0; - int blindTargetCount = 0; switch (this.getAttackStrength()) { case LOW: targetCount = 3; - blindTargetCount = 2; break; case MEDIUM: targetCount = 7; - blindTargetCount = 6; break; case HIGH: targetCount = 13; - blindTargetCount = 12; break; case INSANE: @@ -467,12 +324,6 @@ public void scan(HttpMessage msg, String paramName, String value) { Math.max( PS_PAYLOADS.size(), (Math.max(NIX_OS_PAYLOADS.size(), WIN_OS_PAYLOADS.size()))); - blindTargetCount = - Math.max( - PS_BLIND_PAYLOADS.size(), - (Math.max( - NIX_BLIND_OS_PAYLOADS.size(), - WIN_BLIND_OS_PAYLOADS.size()))); break; default: @@ -480,13 +331,7 @@ public void scan(HttpMessage msg, String paramName, String value) { } if (inScope(Tech.Linux) || inScope(Tech.MacOS)) { - if (testCommandInjection( - paramName, - value, - targetCount, - blindTargetCount, - NIX_OS_PAYLOADS, - NIX_BLIND_OS_PAYLOADS)) { + if (testCommandInjection(paramName, value, targetCount, NIX_OS_PAYLOADS)) { return; } } @@ -497,13 +342,7 @@ public void scan(HttpMessage msg, String paramName, String value) { if (inScope(Tech.Windows)) { // Windows Command Prompt - if (testCommandInjection( - paramName, - value, - targetCount, - blindTargetCount, - WIN_OS_PAYLOADS, - WIN_BLIND_OS_PAYLOADS)) { + if (testCommandInjection(paramName, value, targetCount, WIN_OS_PAYLOADS)) { return; } // Check if the user has stopped the scan @@ -511,15 +350,7 @@ public void scan(HttpMessage msg, String paramName, String value) { return; } // Windows PowerShell - if (testCommandInjection( - paramName, - value, - targetCount, - blindTargetCount, - PS_PAYLOADS, - PS_BLIND_PAYLOADS)) { - return; - } + testCommandInjection(paramName, value, targetCount, PS_PAYLOADS); } } @@ -529,27 +360,19 @@ public void scan(HttpMessage msg, String paramName, String value) { * @param paramName the name of the parameter that will be used for testing for injection * @param value the value of the parameter that will be used for testing for injection * @param targetCount the number of requests for normal payloads - * @param blindTargetCount the number of requests for blind payloads * @param osPayloads the normal payloads - * @param blindOsPayloads the blind payloads * @return {@code true} if the vulnerability was found, {@code false} otherwise. */ private boolean testCommandInjection( - String paramName, - String value, - int targetCount, - int blindTargetCount, - Map osPayloads, - List blindOsPayloads) { - // Start testing OS Command Injection patterns - // ------------------------------------------ + String paramName, String value, int targetCount, Map osPayloads) { + String payload; String paramValue; Iterator it = osPayloads.keySet().iterator(); boolean firstPayload = true; // ----------------------------------------------- - // Check 1: Feedback based OS Command Injection + // Check: Feedback based OS Command Injection // ----------------------------------------------- // try execution check sending a specific payload // and verifying if it returns back the output inside @@ -597,8 +420,7 @@ private boolean testCommandInjection( paramName, paramValue); - buildAlert(paramName, paramValue, matcher.group(), TestType.FEEDBACK, msg) - .raise(); + buildAlert(paramName, paramValue, matcher.group(), msg).raise(); // All done. No need to look for vulnerabilities on subsequent // payloads on the same request (to reduce performance impact) @@ -606,17 +428,12 @@ private boolean testCommandInjection( } } catch (IOException ex) { - // Do not try to internationalise this.. we need an error message in any event.. - // if it's in English, it's still better than not having it at all. LOGGER.warn( "Command Injection vulnerability check failed for parameter [{}] and payload [{}] due to an I/O error", paramName, payload, ex); } - - // Check if the scan has been stopped - // if yes dispose resources and exit if (isStop()) { // Dispose all resources // Exit the scan rule @@ -624,89 +441,6 @@ private boolean testCommandInjection( } } - // ----------------------------------------------- - // Check 2: Time-based Blind OS Command Injection - // ----------------------------------------------- - // Check for a sleep shell execution by using - // linear regression to check for a correlation - // between requested delay and actual delay. - // ----------------------------------------------- - - it = blindOsPayloads.iterator(); - - for (int i = 0; it.hasNext() && (i < blindTargetCount); i++) { - AtomicReference message = new AtomicReference<>(); - String sleepPayload = it.next(); - paramValue = value + sleepPayload.replace("{0}", String.valueOf(timeSleepSeconds)); - - // the function that will send each request - TimingUtils.RequestSender requestSender = - x -> { - HttpMessage msg = getNewMsg(); - message.set(msg); - String finalPayload = - value + sleepPayload.replace("{0}", String.valueOf(x)); - setParameter(msg, paramName, finalPayload); - LOGGER.debug("Testing [{}] = [{}]", paramName, finalPayload); - - // send the request and retrieve the response - sendAndReceive(msg, false); - return msg.getTimeElapsedMillis() / 1000.0; - }; - - boolean isInjectable; - try { - try { - // use TimingUtils to detect a response to sleep payloads - isInjectable = - TimingUtils.checkTimingDependence( - BLIND_REQUESTS_LIMIT, - timeSleepSeconds, - requestSender, - TIME_CORRELATION_ERROR_RANGE, - TIME_SLOPE_ERROR_RANGE); - } catch (SocketException ex) { - LOGGER.debug( - "Caught {} {} when accessing: {}.\n The target may have replied with a poorly formed redirect due to our input.", - ex.getClass().getName(), - ex.getMessage(), - message.get().getRequestHeader().getURI()); - continue; // Something went wrong, move to next blind iteration - } - - if (isInjectable) { - // We Found IT! - // First do logging - LOGGER.debug( - "[Blind OS Command Injection Found] on parameter [{}] with value [{}]", - paramName, - paramValue); - - // just attach this alert to the last sent message - buildAlert(paramName, paramValue, "", TestType.TIME, message.get()).raise(); - - // All done. No need to look for vulnerabilities on subsequent - // payloads on the same request (to reduce performance impact) - return true; - } - } catch (IOException ex) { - // Do not try to internationalise this.. we need an error message in any event.. - // if it's in English, it's still better than not having it at all. - LOGGER.warn( - "Blind Command Injection vulnerability check failed for parameter [{}] and payload [{}] due to an I/O error", - paramName, - paramValue, - ex); - } - - // Check if the scan has been stopped - // if yes dispose resources and exit - if (isStop()) { - // Dispose all resources - // Exit the scan rule - return false; - } - } return false; } @@ -716,7 +450,7 @@ private boolean testCommandInjection( * * @param cmd the cmd to insert uninitialized variable */ - private static String insertUninitVar(String cmd) { + static String insertUninitVar(String cmd) { int varLength = ThreadLocalRandom.current().nextInt(1, 3) + 1; char[] array = new char[varLength]; // $xx @@ -731,23 +465,18 @@ private static String insertUninitVar(String cmd) { .replaceAll("\\/", Matcher.quoteReplacement(var + "/")); } - private AlertBuilder buildAlert( - String param, String attack, String evidence, TestType type, HttpMessage msg) { - String otherInfo = getOtherInfo(type, attack); + AlertBuilder buildAlert(String param, String attack, String evidence, HttpMessage msg) { return newAlert() .setConfidence(Alert.CONFIDENCE_MEDIUM) .setParam(param) .setAttack(attack) .setEvidence(evidence) .setMessage(msg) - .setOtherInfo(otherInfo) - .setTags(getNeededAlertTags(type)); + .setOtherInfo(Constant.messages.getString(MESSAGE_PREFIX + "otherinfo", attack)); } @Override public List getExampleAlerts() { - return List.of( - buildAlert("qry", "a;cat /etc/passwd ", "root:x:0:0", TestType.FEEDBACK, null) - .build()); + return List.of(buildAlert("qry", "a;cat /etc/passwd ", "root:x:0:0", null).build()); } } diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionTimingScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionTimingScanRule.java new file mode 100644 index 00000000000..750db4840c2 --- /dev/null +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionTimingScanRule.java @@ -0,0 +1,329 @@ +/* + * Zed Attack Proxy (ZAP) and its related class files. + * + * ZAP is an HTTP/HTTPS proxy for assessing web application security. + * + * Copyright 2025 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.ascanrules; + +import java.io.IOException; +import java.net.SocketException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.commons.configuration.ConversionException; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.parosproxy.paros.Constant; +import org.parosproxy.paros.core.scanner.Alert; +import org.parosproxy.paros.network.HttpMessage; +import org.zaproxy.addon.commonlib.CommonAlertTag; +import org.zaproxy.addon.commonlib.timing.TimingUtils; +import org.zaproxy.zap.extension.ruleconfig.RuleConfigParam; +import org.zaproxy.zap.model.Tech; + +/** Active scan rule for time based Command Injection testing and verification. */ +public class CommandInjectionTimingScanRule extends CommandInjectionScanRule + implements CommonActiveScanRuleInfo { + + private static final int PLUGIN_ID = 90037; + + /** The name of the rule config to obtain the time, in seconds, for time-based attacks. */ + private static final String RULE_SLEEP_TIME = RuleConfigParam.RULE_COMMON_SLEEP_TIME; + + private static final Map ALERT_TAGS; + + static { + Map alertTags = + new HashMap<>(CommonAlertTag.toMap(CommonAlertTag.TEST_TIMING)); + alertTags.putAll(CommandInjectionScanRule.ALERT_TAGS); + ALERT_TAGS = Collections.unmodifiableMap(alertTags); + } + + /** The default number of seconds used in time-based attacks (i.e. sleep commands). */ + private static final int DEFAULT_TIME_SLEEP_SEC = 5; + + // limit the maximum number of requests sent for time-based attack detection + private static final int BLIND_REQUESTS_LIMIT = 4; + + // error range allowable for statistical time-based blind attacks (0-1.0) + private static final double TIME_CORRELATION_ERROR_RANGE = 0.15; + private static final double TIME_SLOPE_ERROR_RANGE = 0.30; + + // *NIX Blind OS Command constants + private static final String NIX_BLIND_TEST_CMD = "sleep {0}"; + // Windows Blind OS Command constants + private static final String WIN_BLIND_TEST_CMD = "timeout /T {0}"; + // PowerSHell Blind Command constants + private static final String PS_BLIND_TEST_CMD = "start-sleep -s {0}"; + + // OS Command payloads for blind command Injection testing + private static final List NIX_BLIND_OS_PAYLOADS = new LinkedList<>(); + private static final List WIN_BLIND_OS_PAYLOADS = new LinkedList<>(); + private static final List PS_BLIND_PAYLOADS = new LinkedList<>(); + + static { + // No quote payloads + NIX_BLIND_OS_PAYLOADS.add("&" + NIX_BLIND_TEST_CMD + "&"); + NIX_BLIND_OS_PAYLOADS.add(";" + NIX_BLIND_TEST_CMD + ";"); + WIN_BLIND_OS_PAYLOADS.add("&" + WIN_BLIND_TEST_CMD); + WIN_BLIND_OS_PAYLOADS.add("|" + WIN_BLIND_TEST_CMD); + PS_BLIND_PAYLOADS.add(";" + PS_BLIND_TEST_CMD); + + // Double quote payloads + NIX_BLIND_OS_PAYLOADS.add("\"&" + NIX_BLIND_TEST_CMD + "&\""); + NIX_BLIND_OS_PAYLOADS.add("\";" + NIX_BLIND_TEST_CMD + ";\""); + WIN_BLIND_OS_PAYLOADS.add("\"&" + WIN_BLIND_TEST_CMD + "&\""); + WIN_BLIND_OS_PAYLOADS.add("\"|" + WIN_BLIND_TEST_CMD); + PS_BLIND_PAYLOADS.add("\";" + PS_BLIND_TEST_CMD); + + // Single quote payloads + NIX_BLIND_OS_PAYLOADS.add("'&" + NIX_BLIND_TEST_CMD + "&'"); + NIX_BLIND_OS_PAYLOADS.add("';" + NIX_BLIND_TEST_CMD + ";'"); + WIN_BLIND_OS_PAYLOADS.add("'&" + WIN_BLIND_TEST_CMD + "&'"); + WIN_BLIND_OS_PAYLOADS.add("'|" + WIN_BLIND_TEST_CMD); + PS_BLIND_PAYLOADS.add("';" + PS_BLIND_TEST_CMD); + + // Special payloads + NIX_BLIND_OS_PAYLOADS.add("\n" + NIX_BLIND_TEST_CMD + "\n"); // force enter + NIX_BLIND_OS_PAYLOADS.add("`" + NIX_BLIND_TEST_CMD + "`"); // backtick execution + NIX_BLIND_OS_PAYLOADS.add("||" + NIX_BLIND_TEST_CMD); // or control concatenation + NIX_BLIND_OS_PAYLOADS.add("&&" + NIX_BLIND_TEST_CMD); // and control concatenation + NIX_BLIND_OS_PAYLOADS.add("|" + NIX_BLIND_TEST_CMD + "#"); // pipe & comment + // FoxPro for running os commands + WIN_BLIND_OS_PAYLOADS.add("run " + WIN_BLIND_TEST_CMD); + PS_BLIND_PAYLOADS.add(";" + PS_BLIND_TEST_CMD + " #"); // chain & comment + + // uninitialized variable waf bypass + String insertedCMD = CommandInjectionScanRule.insertUninitVar(NIX_BLIND_TEST_CMD); + // No quote payloads + NIX_BLIND_OS_PAYLOADS.add("&" + insertedCMD + "&"); + NIX_BLIND_OS_PAYLOADS.add(";" + insertedCMD + ";"); + // Double quote payloads + NIX_BLIND_OS_PAYLOADS.add("\"&" + insertedCMD + "&\""); + NIX_BLIND_OS_PAYLOADS.add("\";" + insertedCMD + ";\""); + // Single quote payloads + NIX_BLIND_OS_PAYLOADS.add("'&" + insertedCMD + "&'"); + NIX_BLIND_OS_PAYLOADS.add("';" + insertedCMD + ";'"); + // Special payloads + NIX_BLIND_OS_PAYLOADS.add("\n" + insertedCMD + "\n"); + NIX_BLIND_OS_PAYLOADS.add("`" + insertedCMD + "`"); + NIX_BLIND_OS_PAYLOADS.add("||" + insertedCMD); + NIX_BLIND_OS_PAYLOADS.add("&&" + insertedCMD); + NIX_BLIND_OS_PAYLOADS.add("|" + insertedCMD + "#"); + } + + private static final Logger LOGGER = LogManager.getLogger(CommandInjectionTimingScanRule.class); + + /** The number of seconds used in time-based attacks (i.e. sleep commands). */ + private int timeSleepSeconds = DEFAULT_TIME_SLEEP_SEC; + + @Override + public int getId() { + return PLUGIN_ID; + } + + @Override + public String getName() { + return Constant.messages.getString(MESSAGE_PREFIX + "time.name"); + } + + @Override + public void init() { + try { + timeSleepSeconds = this.getConfig().getInt(RULE_SLEEP_TIME, DEFAULT_TIME_SLEEP_SEC); + } catch (ConversionException e) { + LOGGER.debug( + "Invalid value for '{}': {}", + RULE_SLEEP_TIME, + this.getConfig().getString(RULE_SLEEP_TIME)); + } + LOGGER.debug("Sleep set to {} seconds", timeSleepSeconds); + } + + /** + * Gets the number of seconds used in time-based attacks. + * + *

Note: Method provided only to ease the unit tests. + * + * @return the number of seconds used in time-based attacks. + */ + int getTimeSleep() { + return timeSleepSeconds; + } + + @Override + public Map getAlertTags() { + return CommandInjectionTimingScanRule.ALERT_TAGS; + } + + @Override + public void scan(HttpMessage msg, String paramName, String value) { + LOGGER.debug( + "Checking [{}][{}], parameter [{}] for OS Command Injection Vulnerabilities", + msg.getRequestHeader().getMethod(), + msg.getRequestHeader().getURI(), + paramName); + + int blindTargetCount = 0; + switch (this.getAttackStrength()) { + case LOW: + blindTargetCount = 2; + break; + case MEDIUM: + blindTargetCount = 6; + break; + case HIGH: + blindTargetCount = 12; + break; + case INSANE: + blindTargetCount = + Math.max( + PS_BLIND_PAYLOADS.size(), + (Math.max( + NIX_BLIND_OS_PAYLOADS.size(), + WIN_BLIND_OS_PAYLOADS.size()))); + break; + default: + // Default to off + } + + if (inScope(Tech.Linux) || inScope(Tech.MacOS)) { + if (testCommandInjection(paramName, value, blindTargetCount, NIX_BLIND_OS_PAYLOADS)) { + return; + } + } + + if (isStop()) { + return; + } + + if (inScope(Tech.Windows)) { + // Windows Command Prompt + if (testCommandInjection(paramName, value, blindTargetCount, WIN_BLIND_OS_PAYLOADS)) { + return; + } + // Check if the user has stopped the scan + if (isStop()) { + return; + } + // Windows PowerShell + testCommandInjection(paramName, value, blindTargetCount, PS_BLIND_PAYLOADS); + } + } + + /** + * Tests for injection vulnerabilities with the given payloads. + * + * @param paramName the name of the parameter that will be used for testing for injection + * @param value the value of the parameter that will be used for testing for injection + * @param blindTargetCount the number of requests for blind payloads + * @param blindOsPayloads the blind payloads + * @return {@code true} if the vulnerability was found, {@code false} otherwise. + */ + private boolean testCommandInjection( + String paramName, String value, int blindTargetCount, List blindOsPayloads) { + + String paramValue; + + // ----------------------------------------------- + // Check: Time-based Blind OS Command Injection + // ----------------------------------------------- + // Check for a sleep shell execution by using + // linear regression to check for a correlation + // between requested delay and actual delay. + // ----------------------------------------------- + + Iterator it = blindOsPayloads.iterator(); + + for (int i = 0; it.hasNext() && (i < blindTargetCount); i++) { + AtomicReference message = new AtomicReference<>(); + String sleepPayload = it.next(); + paramValue = value + sleepPayload.replace("{0}", String.valueOf(timeSleepSeconds)); + + TimingUtils.RequestSender requestSender = + x -> { + HttpMessage msg = getNewMsg(); + message.set(msg); + String finalPayload = + value + sleepPayload.replace("{0}", String.valueOf(x)); + setParameter(msg, paramName, finalPayload); + LOGGER.debug("Testing [{}] = [{}]", paramName, finalPayload); + + sendAndReceive(msg, false); + return msg.getTimeElapsedMillis() / 1000.0; + }; + + boolean isInjectable; + try { + try { + // use TimingUtils to detect a response to sleep payloads + isInjectable = + TimingUtils.checkTimingDependence( + BLIND_REQUESTS_LIMIT, + timeSleepSeconds, + requestSender, + TIME_CORRELATION_ERROR_RANGE, + TIME_SLOPE_ERROR_RANGE); + } catch (SocketException ex) { + LOGGER.debug( + "Caught {} {} when accessing: {}.\n The target may have replied with a poorly formed redirect due to our input.", + ex.getClass().getName(), + ex.getMessage(), + message.get().getRequestHeader().getURI()); + continue; // Something went wrong, move to next blind iteration + } + + if (isInjectable) { + LOGGER.debug( + "[Blind OS Command Injection Found] on parameter [{}] with value [{}]", + paramName, + paramValue); + + // Attach this alert to the last sent message + buildAlert(paramName, paramValue, message.get()).raise(); + + return true; + } + } catch (IOException ex) { + LOGGER.warn( + "Blind Command Injection vulnerability check failed for parameter [{}] and payload [{}] due to an I/O error", + paramName, + paramValue, + ex); + } + if (isStop()) { + return false; + } + } + return false; + } + + AlertBuilder buildAlert(String param, String attack, HttpMessage msg) { + return buildAlert(param, attack, "", msg) + .setOtherInfo( + Constant.messages.getString(MESSAGE_PREFIX + "time.otherinfo", attack)); + } + + @Override + public List getExampleAlerts() { + return List.of(buildAlert("qry", "sleep 5", null).build()); + } +} diff --git a/addOns/ascanrules/src/main/javahelp/org/zaproxy/zap/extension/ascanrules/resources/help/contents/ascanrules.html b/addOns/ascanrules/src/main/javahelp/org/zaproxy/zap/extension/ascanrules/resources/help/contents/ascanrules.html index cee5efebc82..5e025485c92 100644 --- a/addOns/ascanrules/src/main/javahelp/org/zaproxy/zap/extension/ascanrules/resources/help/contents/ascanrules.html +++ b/addOns/ascanrules/src/main/javahelp/org/zaproxy/zap/extension/ascanrules/resources/help/contents/ascanrules.html @@ -60,21 +60,27 @@

Code Injection


Alert ID: 90019. -

Command Injection

+

Remote OS Command Injection

-This rule submits *NIX and Windows OS commands as URL parameter values to determine whether or not the web application is passing unchecked +This rule injects *NIX and Windows OS commands to determine whether or not the web application is passing unchecked user input directly to the underlying OS. The injection strings consist of meta-characters that may be interpreted by the OS -as join commands along with a payload that should generate output in the response if the application is vulnerable. If the content of a response body -matches the payload, the scanner raises an alert and returns immediately. In the event that none of the error-based matching attempts -return output in the response, the scanner will attempt a blind injection attack by submitting sleep instructions as the payload and comparing the elapsed time between sending the request -and receiving the response against a heuristic time-delay lower limit. If the elapsed time is greater than this limit, an alert is raised with medium confidence -and the scanner returns immediately. +as join commands along with a payload that should generate output in the response if the application is vulnerable. +

+Latest code: CommandInjectionScanRule.java +
+Alert ID: 90020. + +

Remote OS Command Injection (Time Based)

+ +This rule injects *NIX and Windows OS commands to determine whether or not the web application is passing unchecked +user input directly to the underlying OS. The rule will attempt blind injection attack(s) by submitting sleep instructions as the payload and comparing the elapsed time between sending the request +and receiving the response against a heuristic time-delay lower limit.
Post 2.5.0 you can change the length of time used for the blind injection attack by changing the rules.common.sleep parameter via the Options 'Rule configuration' panel.

-Latest code: CommandInjectionScaRule.java +Latest code: CommandInjectionTimingScanRule.java
-Alert ID: 90020. +Alert ID: 90037.

Cross Site Scripting (Reflected)

diff --git a/addOns/ascanrules/src/main/resources/org/zaproxy/zap/extension/ascanrules/resources/Messages.properties b/addOns/ascanrules/src/main/resources/org/zaproxy/zap/extension/ascanrules/resources/Messages.properties index 4007d37af80..58a2188af25 100644 --- a/addOns/ascanrules/src/main/resources/org/zaproxy/zap/extension/ascanrules/resources/Messages.properties +++ b/addOns/ascanrules/src/main/resources/org/zaproxy/zap/extension/ascanrules/resources/Messages.properties @@ -20,10 +20,12 @@ ascanrules.codeinjection.soln = Do not trust client side input, even if there is ascanrules.commandinjection.desc = Attack technique used for unauthorized execution of operating system commands. This attack is possible when an application accepts untrusted input to build operating system commands in an insecure manner involving improper data sanitization, and/or improper calling of external programs. ascanrules.commandinjection.name = Remote OS Command Injection -ascanrules.commandinjection.otherinfo.feedback-based = The scan rule was able to retrieve the content of a file or command by sending [{0}] to the operating system running this application. -ascanrules.commandinjection.otherinfo.time-based = The scan rule was able to control the timing of the application response by sending [{0}] to the operating system running this application. +ascanrules.commandinjection.otherinfo = The scan rule was able to retrieve the content of a file or command by sending [{0}] to the operating system running this application. ascanrules.commandinjection.refs = https://cwe.mitre.org/data/definitions/78.html\nhttps://owasp.org/www-community/attacks/Command_Injection +ascanrules.commandinjection.time.name = Remote OS Command Injection (Time Based) +ascanrules.commandinjection.time.otherinfo = The scan rule was able to control the timing of the application response by sending [{0}] to the operating system running this application. + ascanrules.crlfinjection.desc = Cookie can be set via CRLF injection. It may also be possible to set arbitrary HTTP response headers. In addition, by carefully crafting the injected response using cross-site script, cache poisoning vulnerability may also exist. ascanrules.crlfinjection.name = CRLF Injection ascanrules.crlfinjection.refs = https://owasp.org/www-community/vulnerabilities/CRLF_Injection\nhttps://cwe.mitre.org/data/definitions/113.html diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionRuleTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionRuleTest.java new file mode 100644 index 00000000000..773eee37399 --- /dev/null +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionRuleTest.java @@ -0,0 +1,85 @@ +/* + * Zed Attack Proxy (ZAP) and its related class files. + * + * ZAP is an HTTP/HTTPS proxy for assessing web application security. + * + * Copyright 2025 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.ascanrules; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +import org.junit.jupiter.api.Test; +import org.parosproxy.paros.core.scanner.AbstractPlugin; +import org.zaproxy.zap.model.Tech; +import org.zaproxy.zap.model.TechSet; +import org.zaproxy.zap.testutils.ActiveScannerTestUtils; + +abstract class CommandInjectionRuleTest + extends ActiveScannerTestUtils { + + @Override + protected void setUpMessages() { + mockMessages(new ExtensionAscanRules()); + } + + @Test + void shouldTargetLinuxTech() { + // Given + TechSet techSet = techSet(Tech.Linux); + // When + boolean targets = rule.targets(techSet); + // Then + assertThat(targets, is(equalTo(true))); + } + + @Test + void shouldTargetMacOsTech() { + // Given + TechSet techSet = techSet(Tech.MacOS); + // When + boolean targets = rule.targets(techSet); + // Then + assertThat(targets, is(equalTo(true))); + } + + @Test + void shouldTargetWindowsTech() { + // Given + TechSet techSet = techSet(Tech.Windows); + // When + boolean targets = rule.targets(techSet); + // Then + assertThat(targets, is(equalTo(true))); + } + + @Test + void shouldNotTargetNonLinuxMacOsWindowsTechs() { + // Given + TechSet techSet = techSetWithout(Tech.Linux, Tech.MacOS, Tech.Windows); + // When + boolean targets = rule.targets(techSet); + // Then + assertThat(targets, is(equalTo(false))); + } + + @Test + @Override + public void shouldHaveValidReferences() { + super.shouldHaveValidReferences(); + } +} diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRuleUnitTest.java index 7aff96ae92c..a0b9205c2fd 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRuleUnitTest.java @@ -21,70 +21,53 @@ import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; -import fi.iki.elonen.NanoHTTPD; import fi.iki.elonen.NanoHTTPD.IHTTPSession; import fi.iki.elonen.NanoHTTPD.Response; -import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.function.Predicate; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Stream; -import org.apache.commons.configuration.Configuration; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.parosproxy.paros.core.scanner.Alert; -import org.parosproxy.paros.core.scanner.Plugin; import org.parosproxy.paros.core.scanner.Plugin.AttackStrength; import org.parosproxy.paros.network.HttpMalformedHeaderException; -import org.parosproxy.paros.network.HttpMessage; import org.zaproxy.addon.commonlib.CommonAlertTag; import org.zaproxy.addon.commonlib.PolicyTag; -import org.zaproxy.zap.extension.ruleconfig.RuleConfigParam; import org.zaproxy.zap.model.Tech; -import org.zaproxy.zap.model.TechSet; import org.zaproxy.zap.testutils.NanoServerHandler; -import org.zaproxy.zap.utils.ZapXmlConfiguration; /** Unit test for {@link CommandInjectionScanRule}. */ -class CommandInjectionScanRuleUnitTest extends ActiveScannerTest { +class CommandInjectionScanRuleUnitTest extends CommandInjectionRuleTest { @Override protected int getRecommendMaxNumberMessagesPerParam(AttackStrength strength) { int recommendMax = super.getRecommendMaxNumberMessagesPerParam(strength); switch (strength) { case LOW: - return recommendMax + 9; + return recommendMax + 3; case MEDIUM: default: - return recommendMax + 23; + return recommendMax + 7; case HIGH: - return recommendMax + 30; + return recommendMax + 7; case INSANE: - return recommendMax + 17; + return recommendMax; } } @Override protected CommandInjectionScanRule createScanner() { - CommandInjectionScanRule scanner = new CommandInjectionScanRule(); - scanner.setConfig(new ZapXmlConfiguration()); - return scanner; + return new CommandInjectionScanRule(); } @Test @@ -96,7 +79,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(78))); assertThat(wasc, is(equalTo(31))); - assertThat(tags.size(), is(equalTo(14))); + assertThat(tags.size(), is(equalTo(13))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(true))); @@ -108,7 +91,6 @@ void shouldReturnExpectedMappings() { is(equalTo(true))); assertThat(tags.containsKey(CommonAlertTag.HIPAA.getTag()), is(equalTo(true))); assertThat(tags.containsKey(CommonAlertTag.PCI_DSS.getTag()), is(equalTo(true))); - assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.API.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.DEV_CICD.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.DEV_STD.getTag()), is(equalTo(true))); @@ -128,118 +110,6 @@ void shouldReturnExpectedMappings() { is(equalTo(CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ.getValue()))); } - @Test - void shouldTargetLinuxTech() { - // Given - TechSet techSet = techSet(Tech.Linux); - // When - boolean targets = rule.targets(techSet); - // Then - assertThat(targets, is(equalTo(true))); - } - - @Test - void shouldTargetMacOsTech() { - // Given - TechSet techSet = techSet(Tech.MacOS); - // When - boolean targets = rule.targets(techSet); - // Then - assertThat(targets, is(equalTo(true))); - } - - @Test - void shouldTargetWindowsTech() { - // Given - TechSet techSet = techSet(Tech.Windows); - // When - boolean targets = rule.targets(techSet); - // Then - assertThat(targets, is(equalTo(true))); - } - - @Test - void shouldNotTargetNonLinuxMacOsWindowsTechs() { - // Given - TechSet techSet = techSetWithout(Tech.Linux, Tech.MacOS, Tech.Windows); - // When - boolean targets = rule.targets(techSet); - // Then - assertThat(targets, is(equalTo(false))); - } - - @Test - void shouldFailToInitWithoutConfig() throws Exception { - // Given - CommandInjectionScanRule scanner = new CommandInjectionScanRule(); - HttpMessage msg = getHttpMessage(""); - // When / Then - assertThrows(NullPointerException.class, () -> scanner.init(msg, parent)); - } - - @Test - void shouldInitWithConfig() throws Exception { - // Given - CommandInjectionScanRule scanner = new CommandInjectionScanRule(); - scanner.setConfig(new ZapXmlConfiguration()); - // When / Then - assertDoesNotThrow(() -> scanner.init(getHttpMessage(""), parent)); - } - - @Test - void shouldUse5SecsByDefaultForTimeBasedAttacks() throws Exception { - // Given / When - int time = rule.getTimeSleep(); - // Then - assertThat(time, is(equalTo(5))); - } - - @Test - void shouldUseTimeDefinedInConfigForTimeBasedAttacks() throws Exception { - // Given - rule.setConfig(configWithSleepRule("10")); - // When - rule.init(getHttpMessage(""), parent); - // Then - assertThat(rule.getTimeSleep(), is(equalTo(10))); - } - - @Test - void shouldDefaultTo5SecsIfConfigTimeIsMalformedValueForTimeBasedAttacks() throws Exception { - // Given - rule.setConfig(configWithSleepRule("not a valid value")); - // When - rule.init(getHttpMessage(""), parent); - // Then - assertThat(rule.getTimeSleep(), is(equalTo(5))); - } - - @Test - void shouldUseSpecifiedTimeInAllTimeBasedPayloads() throws Exception { - // Given - String sleepTime = "987"; - PayloadCollectorHandler payloadCollector = - new PayloadCollectorHandler( - "/", "p", v -> v.contains("sleep") || v.contains("timeout")); - nano.addHandler(payloadCollector); - rule.setConfig(configWithSleepRule(sleepTime)); - rule.setAttackStrength(Plugin.AttackStrength.INSANE); - rule.init(getHttpMessage("?p=v"), parent); - // When - rule.scan(); - // Then - for (String payload : payloadCollector.getPayloads()) { - assertThat(payload, not(containsString("{0}"))); - assertThat(payload, containsString(sleepTime)); - } - } - - private static Configuration configWithSleepRule(String value) { - Configuration config = new ZapXmlConfiguration(); - config.setProperty(RuleConfigParam.RULE_COMMON_SLEEP_TIME, value); - return config; - } - @Test void shouldRaiseAlertIfResponseHasPasswdFileContentAndPayloadIsNullByteBased() throws HttpMalformedHeaderException { @@ -303,40 +173,6 @@ protected Response serve(IHTTPSession session) { assertThat(alertsRaised, hasSize(1)); } - @Test - void shouldDetectTimeBasedInjection() throws HttpMalformedHeaderException { - // Given - Pattern sleepPattern = Pattern.compile("(?:sleep|timeout /T|start-sleep -s) (\\d+)"); - String regularContent = "Nothing to see here."; - nano.addHandler( - new NanoServerHandler("/") { - @Override - protected Response serve(IHTTPSession session) { - String value = getFirstParamValue(session, "p"); - if (value == null) { - return newFixedLengthResponse(regularContent); - } - Matcher match = sleepPattern.matcher(value); - if (!match.find()) { - return newFixedLengthResponse(regularContent); - } - try { - int sleepInput = Integer.parseInt(match.group(1)); - Thread.sleep(sleepInput * 1000L); - } catch (InterruptedException ex) { - fail("failed to sleep thread for time-based command injection"); - } - return newFixedLengthResponse(regularContent); - } - }); - rule.init(getHttpMessage("/?p=a"), parent); - // When - rule.scan(); - // Then - assertThat(alertsRaised, hasSize(1)); - assertThat(sleepPattern.matcher(alertsRaised.get(0).getAttack()).find(), is(true)); - } - private static Stream shouldReturnRelevantTechs() { return Stream.of( Arguments.of(Tech.Windows), Arguments.of(Tech.Linux), Arguments.of(Tech.MacOS)); @@ -359,12 +195,6 @@ void firstPayloadShouldNotHaveParamValue(Tech targetedTech) assertTrue(httpMessagesSent.get(1).getUrlParams().first().getValue().startsWith("a")); } - @Test - @Override - public void shouldHaveValidReferences() { - super.shouldHaveValidReferences(); - } - @Test void shouldHaveExpectedExampleAlert() { // Given / WHen @@ -386,33 +216,4 @@ void shouldHaveExpectedExampleAlert() { Map tags = alert.getTags(); assertThat(tags, not(hasKey(CommonAlertTag.TEST_TIMING.getTag()))); } - - private static class PayloadCollectorHandler extends NanoServerHandler { - - private final String param; - private final Predicate valuePredicate; - private final List payloads; - - public PayloadCollectorHandler( - String path, String param, Predicate valuePredicate) { - super(path); - - this.param = param; - this.valuePredicate = valuePredicate; - this.payloads = new ArrayList<>(); - } - - public List getPayloads() { - return payloads; - } - - @Override - protected Response serve(IHTTPSession session) { - String value = getFirstParamValue(session, param); - if (valuePredicate.test(value)) { - payloads.add(value); - } - return newFixedLengthResponse(Response.Status.OK, NanoHTTPD.MIME_HTML, "Content"); - } - } } diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionTimingScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionTimingScanRuleUnitTest.java new file mode 100644 index 00000000000..90a757337a4 --- /dev/null +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionTimingScanRuleUnitTest.java @@ -0,0 +1,278 @@ +/* + * Zed Attack Proxy (ZAP) and its related class files. + * + * ZAP is an HTTP/HTTPS proxy for assessing web application security. + * + * Copyright 2025 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.ascanrules; + +import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +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 static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; + +import fi.iki.elonen.NanoHTTPD; +import fi.iki.elonen.NanoHTTPD.IHTTPSession; +import fi.iki.elonen.NanoHTTPD.Response; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.function.Predicate; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.apache.commons.configuration.Configuration; +import org.junit.jupiter.api.Test; +import org.parosproxy.paros.core.scanner.Alert; +import org.parosproxy.paros.core.scanner.Plugin; +import org.parosproxy.paros.core.scanner.Plugin.AttackStrength; +import org.parosproxy.paros.network.HttpMalformedHeaderException; +import org.parosproxy.paros.network.HttpMessage; +import org.zaproxy.addon.commonlib.CommonAlertTag; +import org.zaproxy.addon.commonlib.PolicyTag; +import org.zaproxy.zap.extension.ruleconfig.RuleConfigParam; +import org.zaproxy.zap.testutils.NanoServerHandler; +import org.zaproxy.zap.utils.ZapXmlConfiguration; + +/** Unit test for {@link CommandInjectionTimingScanRule}. */ +class CommandInjectionTimingScanRuleUnitTest + extends CommandInjectionRuleTest { + + @Override + protected int getRecommendMaxNumberMessagesPerParam(AttackStrength strength) { + int recommendMax = super.getRecommendMaxNumberMessagesPerParam(strength); + switch (strength) { + case MEDIUM: + return recommendMax + 4; + default: + return recommendMax; + } + } + + @Override + protected CommandInjectionTimingScanRule createScanner() { + CommandInjectionTimingScanRule scanRule = new CommandInjectionTimingScanRule(); + scanRule.setConfig(new ZapXmlConfiguration()); + return scanRule; + } + + @Test + void shouldReturnExpectedMappings() { + // Given / When + int cwe = rule.getCweId(); + int wasc = rule.getWascId(); + Map tags = rule.getAlertTags(); + // Then + assertThat(cwe, is(equalTo(78))); + assertThat(wasc, is(equalTo(31))); + assertThat(tags.size(), is(equalTo(14))); + assertThat( + tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), + is(equalTo(true))); + assertThat( + tags.containsKey(CommonAlertTag.OWASP_2017_A01_INJECTION.getTag()), + is(equalTo(true))); + assertThat( + tags.containsKey(CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ.getTag()), + is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.HIPAA.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.PCI_DSS.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(PolicyTag.API.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(PolicyTag.DEV_CICD.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(PolicyTag.DEV_STD.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(PolicyTag.DEV_FULL.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(PolicyTag.QA_STD.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(PolicyTag.SEQUENCE.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(PolicyTag.PENTEST.getTag()), is(equalTo(true))); + assertThat( + tags.get(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), + is(equalTo(CommonAlertTag.OWASP_2021_A03_INJECTION.getValue()))); + assertThat( + tags.get(CommonAlertTag.OWASP_2017_A01_INJECTION.getTag()), + is(equalTo(CommonAlertTag.OWASP_2017_A01_INJECTION.getValue()))); + assertThat( + tags.get(CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ.getTag()), + is(equalTo(CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ.getValue()))); + } + + @Test + void shouldFailToInitWithoutConfig() throws Exception { + // Given + CommandInjectionTimingScanRule scanRule = new CommandInjectionTimingScanRule(); + HttpMessage msg = getHttpMessage(""); + // When / Then + assertThrows(NullPointerException.class, () -> scanRule.init(msg, parent)); + } + + @Test + void shouldInitWithConfig() throws Exception { + // Given + CommandInjectionTimingScanRule scanRule = new CommandInjectionTimingScanRule(); + scanRule.setConfig(new ZapXmlConfiguration()); + // When / Then + assertDoesNotThrow(() -> scanRule.init(getHttpMessage(""), parent)); + } + + @Test + void shouldUse5SecsByDefaultForTimeBasedAttacks() { + // Given / When + int time = rule.getTimeSleep(); + // Then + assertThat(time, is(equalTo(5))); + } + + @Test + void shouldUseTimeDefinedInConfigForTimeBasedAttacks() throws Exception { + // Given + rule.setConfig(configWithSleepRule("10")); + // When + rule.init(getHttpMessage(""), parent); + // Then + assertThat(rule.getTimeSleep(), is(equalTo(10))); + } + + @Test + void shouldDefaultTo5SecsIfConfigTimeIsMalformedValueForTimeBasedAttacks() throws Exception { + // Given + rule.setConfig(configWithSleepRule("not a valid value")); + // When + rule.init(getHttpMessage(""), parent); + // Then + assertThat(rule.getTimeSleep(), is(equalTo(5))); + } + + @Test + void shouldUseSpecifiedTimeInAllTimeBasedPayloads() throws Exception { + // Given + String sleepTime = "987"; + PayloadCollectorHandler payloadCollector = + new PayloadCollectorHandler( + "/", "p", v -> v.contains("sleep") || v.contains("timeout")); + nano.addHandler(payloadCollector); + rule.setConfig(configWithSleepRule(sleepTime)); + rule.setAttackStrength(Plugin.AttackStrength.INSANE); + rule.init(getHttpMessage("?p=v"), parent); + // When + rule.scan(); + // Then + for (String payload : payloadCollector.getPayloads()) { + assertThat(payload, not(containsString("{0}"))); + assertThat(payload, containsString(sleepTime)); + } + } + + private static Configuration configWithSleepRule(String value) { + Configuration config = new ZapXmlConfiguration(); + config.setProperty(RuleConfigParam.RULE_COMMON_SLEEP_TIME, value); + return config; + } + + @Test + void shouldDetectTimeBasedInjection() throws HttpMalformedHeaderException { + // Given + Pattern sleepPattern = Pattern.compile("(?:sleep|timeout /T|start-sleep -s) (\\d+)"); + String regularContent = "Nothing to see here."; + nano.addHandler( + new NanoServerHandler("/") { + @Override + protected Response serve(IHTTPSession session) { + String value = getFirstParamValue(session, "p"); + if (value == null) { + return newFixedLengthResponse(regularContent); + } + Matcher match = sleepPattern.matcher(value); + if (!match.find()) { + return newFixedLengthResponse(regularContent); + } + try { + int sleepInput = Integer.parseInt(match.group(1)); + Thread.sleep(sleepInput * 1000L); + } catch (InterruptedException ex) { + fail("failed to sleep thread for time-based command injection"); + } + return newFixedLengthResponse(regularContent); + } + }); + rule.init(getHttpMessage("/?p=a"), parent); + // When + rule.scan(); + // Then + assertThat(alertsRaised, hasSize(1)); + assertThat(sleepPattern.matcher(alertsRaised.get(0).getAttack()).find(), is(true)); + } + + @Test + @Override + public void shouldHaveValidReferences() { + super.shouldHaveValidReferences(); + } + + @Test + void shouldHaveExpectedExampleAlert() { + // Given / WHen + List alerts = rule.getExampleAlerts(); + // Then + assertThat(alerts.size(), is(equalTo(1))); + Alert alert = alerts.get(0); + assertThat(alert.getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM))); + assertThat(alert.getParam(), is(equalTo("qry"))); + assertThat(alert.getAttack(), is(equalTo("sleep 5"))); + assertThat( + alert.getOtherInfo(), + is( + equalTo( + "The scan rule was able to control the timing " + + "of the application response by sending " + + "[sleep 5] to the operating system running " + + "this application."))); + } + + private static class PayloadCollectorHandler extends NanoServerHandler { + + private final String param; + private final Predicate valuePredicate; + private final List payloads; + + public PayloadCollectorHandler( + String path, String param, Predicate valuePredicate) { + super(path); + + this.param = param; + this.valuePredicate = valuePredicate; + this.payloads = new ArrayList<>(); + } + + public List getPayloads() { + return payloads; + } + + @Override + protected Response serve(IHTTPSession session) { + String value = getFirstParamValue(session, param); + if (valuePredicate.test(value)) { + payloads.add(value); + } + return newFixedLengthResponse(Response.Status.OK, NanoHTTPD.MIME_HTML, "Content"); + } + } +}