diff --git a/addOns/ascanrules/CHANGELOG.md b/addOns/ascanrules/CHANGELOG.md index 12b9b3b58ce..9714987c485 100644 --- a/addOns/ascanrules/CHANGELOG.md +++ b/addOns/ascanrules/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [82] - 2026-05-06 ### Changed +- XML External Entity Attack scan rule extended to detect XXE attacks when XML is part of a multipart request (Issue 1190). - The following scan rules now include example alert functionality for documentation generation purposes (Issue 6119): - SQL Injection - Hypersonic SQL (Time Based) - SQL Injection - MsSQL (Time Based) diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XxeScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XxeScanRule.java index 968da4e2352..c0b71dcb2f5 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XxeScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XxeScanRule.java @@ -23,17 +23,20 @@ import java.text.MessageFormat; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.parosproxy.paros.Constant; import org.parosproxy.paros.control.Control; -import org.parosproxy.paros.core.scanner.AbstractAppPlugin; +import org.parosproxy.paros.core.scanner.AbstractAppParamPlugin; import org.parosproxy.paros.core.scanner.Alert; import org.parosproxy.paros.core.scanner.Category; +import org.parosproxy.paros.core.scanner.NameValuePair; import org.parosproxy.paros.network.HttpMessage; import org.zaproxy.addon.commonlib.CommonAlertTag; import org.zaproxy.addon.commonlib.PolicyTag; @@ -47,7 +50,7 @@ * * @author yhawke (2104) */ -public class XxeScanRule extends AbstractAppPlugin implements CommonActiveScanRuleInfo { +public class XxeScanRule extends AbstractAppParamPlugin implements CommonActiveScanRuleInfo { private static final String MESSAGE_PREFIX = "ascanrules.xxe."; private static final int PLUGIN_ID = 90023; @@ -129,6 +132,8 @@ public class XxeScanRule extends AbstractAppPlugin implements CommonActiveScanRu // Logger instance private static final Logger LOGGER = LogManager.getLogger(XxeScanRule.class); + private Set xmlFileNames = Set.of(); + private boolean alertRaised = false; @Override public int getId() { @@ -182,45 +187,85 @@ public int getRisk() { /** * Scan rule to check for XXE vulnerabilities. It checks both for local and remote using the ZAP - * API and also a new model based on parameter substitution + * API and also a new model based on parameter substitution. + * + *

For direct XML requests, the whole body is attacked. For multipart/form-data requests, the + * variant framework is used so that the user's input vector selection is respected. */ @Override public void scan() { - // Prepare the message + alertRaised = false; + HttpMessage msg = getBaseMsg(); String contentType = msg.getRequestHeader().getHeader(HttpFieldsNames.CONTENT_TYPE); - // first check if it's an XML otherwise it's useless... if ((contentType != null) && (contentType.contains("xml"))) { + // Direct XML body — attack the whole body + scanForXxe(null); + } else { + // Let the framework iterate variants/parameters + super.scan(); + } + } - // Check #1 : XXE Remote File Inclusion Attack - remoteFileInclusionAttack(); + @Override + protected void scan(List nameValuePairs) { + xmlFileNames = new HashSet<>(); + for (NameValuePair p : nameValuePairs) { + if (p.getType() == NameValuePair.TYPE_MULTIPART_DATA_FILE_CONTENTTYPE + && p.getValue() != null + && p.getValue().toLowerCase().contains("xml")) { + xmlFileNames.add(p.getName()); + } + } + super.scan(nameValuePairs); + } - // Check #2 : Out-of-band XXE Attack - outOfBandFileInclusionAttack(); + @Override + public void scan(HttpMessage msg, NameValuePair originalParam) { + if (alertRaised || isStop()) { + return; + } + if (originalParam.getType() == NameValuePair.TYPE_MULTIPART_DATA_FILE_PARAM + && xmlFileNames.contains(originalParam.getName())) { + scanForXxe(originalParam); + } + } - // Check if we've to do only basic analysis (only remote should be done)... - if (this.getAttackStrength() == AttackStrength.LOW) { - return; - } + @Override + public void scan(HttpMessage msg, String param, String value) { + // Not used — XXE is handled at body level in scan() + // or via NameValuePair for multipart parameters. + } - // Check #3 : XXE Local File Reflection Attack - localFileReflectionAttack(getNewMsg()); + private void scanForXxe(NameValuePair originalPair) { + // Check #1 : XXE Remote File Inclusion Attack + remoteFileInclusionAttack(originalPair); - // Check if we've to do only medium sized analysis - // (only remote and reflected will be done) - if (this.getAttackStrength() == AttackStrength.MEDIUM) { - return; - } + // Check #2 : Out-of-band XXE Attack + outOfBandFileInclusionAttack(originalPair); - // Exit if the scan has been stopped - if (isStop()) { - return; - } + // Check if we've to do only basic analysis (only remote should be done)... + if (this.getAttackStrength() == AttackStrength.LOW) { + return; + } + + // Check #3 : XXE Local File Reflection Attack + localFileReflectionAttack(getNewMsg(), originalPair); - // Check #4 : XXE Local File Inclusion Attack - localFileInclusionAttack(getNewMsg()); + // Check if we've to do only medium sized analysis + // (only remote and reflected will be done) + if (this.getAttackStrength() == AttackStrength.MEDIUM) { + return; } + + // Exit if the scan has been stopped + if (isStop()) { + return; + } + + // Check #4 : XXE Local File Inclusion Attack + localFileInclusionAttack(getNewMsg(), originalPair); } /** @@ -229,7 +274,7 @@ public void scan() { * external bouncing site, in this case we use the ZAP API as a server for the vulnerability * check using a challenge/response model based on a random string */ - private void remoteFileInclusionAttack() { + private void remoteFileInclusionAttack(NameValuePair originalPair) { try { ExtensionOast extOast = Control.getSingleton().getExtensionLoader().getExtension(ExtensionOast.class); @@ -246,7 +291,11 @@ private void remoteFileInclusionAttack() { alert, XxeScanRule.class.getSimpleName()); String payload = MessageFormat.format(ATTACK_MESSAGE, callbackPayload); alert.setAttack(payload); - msg.setRequestBody(payload); + if (originalPair != null) { + setParameter(msg, originalPair.getName(), payload); + } else { + msg.setRequestBody(payload); + } sendAndReceive(msg); } } catch (Exception e) { @@ -254,7 +303,7 @@ private void remoteFileInclusionAttack() { } } - private void outOfBandFileInclusionAttack() { + private void outOfBandFileInclusionAttack(NameValuePair originalPair) { try { ExtensionOast extOast = Control.getSingleton().getExtensionLoader().getExtension(ExtensionOast.class); @@ -269,12 +318,20 @@ private void outOfBandFileInclusionAttack() { String oastPayload = extOast.registerAlertAndGetPayload(alert); String payload = MessageFormat.format(ATTACK_MESSAGE, "http://" + oastPayload); alert.setAttack(payload); - msg.setRequestBody(payload); + if (originalPair != null) { + setParameter(msg, originalPair.getName(), payload); + } else { + msg.setRequestBody(payload); + } sendAndReceive(msg); // Try again with https msg = getNewMsg(); payload = MessageFormat.format(ATTACK_MESSAGE, "https://" + oastPayload); - msg.setRequestBody(payload); + if (originalPair != null) { + setParameter(msg, originalPair.getName(), payload); + } else { + msg.setRequestBody(payload); + } sendAndReceive(msg); } } catch (Exception e) { @@ -292,19 +349,21 @@ private void outOfBandFileInclusionAttack() { * * @param msg new HttpMessage with the same request as the base. This is used to build the * attack payload. + * @param originalPair the multipart parameter to attack, or null for direct XML body */ - private void localFileReflectionAttack(HttpMessage msg) { - // First replace the values in all the Elements by the Attack Entity - String originalRequestBody = msg.getRequestBody().toString(); - String requestBody = createLfrPayload(originalRequestBody); - if (localFileReflectionTest(msg, requestBody)) { + private void localFileReflectionAttack(HttpMessage msg, NameValuePair originalPair) { + String originalXml; + if (originalPair != null) { + originalXml = originalPair.getValue(); + } else { + originalXml = msg.getRequestBody().toString(); + } + String requestBody = createLfrPayload(originalXml); + if (localFileReflectionTest(requestBody, originalPair)) { return; } - // Now if no issue is found yet, then we replace the values one at a time. Do this for a - // fixed number of Elements, depending on the strength at which the rule is used. - // Remove original xml header - Matcher headerMatcher = xmlHeaderPattern.matcher(originalRequestBody); + Matcher headerMatcher = xmlHeaderPattern.matcher(originalXml); String headerlessRequestBody = headerMatcher.replaceAll(""); int maxValuesChanged = 0; @@ -318,7 +377,7 @@ private void localFileReflectionAttack(HttpMessage msg) { Matcher tagMatcher = tagPattern.matcher(headerlessRequestBody); for (int tagIdx = 1; (tagIdx <= maxValuesChanged) && tagMatcher.find(); tagIdx++) { requestBody = createTagSpecificLfrPayload(headerlessRequestBody, tagMatcher); - if (localFileReflectionTest(msg, requestBody)) { + if (localFileReflectionTest(requestBody, originalPair)) { return; } } @@ -337,27 +396,32 @@ private void localFileReflectionAttack(HttpMessage msg) { * * @param msg new HttpMessage with the same request as the base. This is used to build the * attack payload. + * @param originalPair the multipart parameter to attack, or null for direct XML body */ - private void localFileInclusionAttack(HttpMessage msg) { + private void localFileInclusionAttack(HttpMessage msg, NameValuePair originalPair) { String payload = null; try { for (int idx = 0; idx < LOCAL_FILE_TARGETS.length; idx++) { + msg = getNewMsg(); String localFile = LOCAL_FILE_TARGETS[idx]; payload = MessageFormat.format(ATTACK_MESSAGE, localFile); - msg.setRequestBody(payload); + if (originalPair != null) { + setParameter(msg, originalPair.getName(), payload); + } else { + msg.setRequestBody(payload); + } sendAndReceive(msg); String response = msg.getResponseBody().toString(); Matcher matcher = LOCAL_FILE_PATTERNS[idx].matcher(response); if (matcher.find()) { createAlert(payload, matcher.group()).setMessage(msg).raise(); + alertRaised = true; } if (isStop()) { return; } } } 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( "XXE Injection vulnerability check failed for payload [{}] due to an I/O error", payload, @@ -385,11 +449,16 @@ static String createLfrPayload(String requestBody) { return sb.toString(); } - private boolean localFileReflectionTest(HttpMessage msg, String requestBody) { + private boolean localFileReflectionTest(String requestBody, NameValuePair originalPair) { for (int idx = 0; idx < LOCAL_FILE_TARGETS.length; idx++) { + HttpMessage msg = getNewMsg(); String localFile = LOCAL_FILE_TARGETS[idx]; String payload = MessageFormat.format(requestBody, localFile); - msg.setRequestBody(payload); + if (originalPair != null) { + setParameter(msg, originalPair.getName(), payload); + } else { + msg.setRequestBody(payload); + } try { sendAndReceive(msg); } catch (IOException ex) { @@ -403,6 +472,7 @@ private boolean localFileReflectionTest(HttpMessage msg, String requestBody) { Matcher matcher = LOCAL_FILE_PATTERNS[idx].matcher(response); if (matcher.find()) { createAlert(payload, matcher.group()).setMessage(msg).raise(); + alertRaised = true; return true; } if (isStop()) {