From 34ce817a3f865e96ee7f8609d6669f5962841997 Mon Sep 17 00:00:00 2001 From: Cedric Buissart Date: Tue, 5 May 2026 19:30:46 +0000 Subject: [PATCH 1/2] feat: add multipart/form-data XXE detection support Port logic from PR #2864 to detect XXE vulnerabilities when XML content is uploaded as part of a multipart/form-data request. Uses VariantMultipartFormParameters to parse and inject payloads into multipart bodies while preserving other form parts. Includes import reorder and state reset fix for scanner reuse. Signed-off-by: Cedric Buissart --- addOns/ascanrules/CHANGELOG.md | 1 + .../zap/extension/ascanrules/XxeScanRule.java | 171 ++++++++++-------- 2 files changed, 98 insertions(+), 74 deletions(-) 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..664b4f64c22 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 @@ -27,6 +27,7 @@ import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.parosproxy.paros.Constant; @@ -34,6 +35,8 @@ import org.parosproxy.paros.core.scanner.AbstractAppPlugin; 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.core.scanner.VariantMultipartFormParameters; import org.parosproxy.paros.network.HttpMessage; import org.zaproxy.addon.commonlib.CommonAlertTag; import org.zaproxy.addon.commonlib.PolicyTag; @@ -129,6 +132,8 @@ public class XxeScanRule extends AbstractAppPlugin implements CommonActiveScanRu // Logger instance private static final Logger LOGGER = LogManager.getLogger(XxeScanRule.class); + private VariantMultipartFormParameters multipartVariant; + private boolean alertRaised = false; @Override public int getId() { @@ -180,47 +185,63 @@ public int getRisk() { return Alert.RISK_HIGH; } - /** - * 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 - */ @Override public void scan() { - // Prepare the message + multipartVariant = null; + 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"))) { + scanForXxe(null); + } else if ((contentType != null) && (contentType.contains("multipart"))) { + multipartVariant = new VariantMultipartFormParameters(); + multipartVariant.setMessage(getNewMsg()); + List xmlFileNames = + multipartVariant.getParamList().stream() + .filter( + p -> + (p.getType() + == NameValuePair + .TYPE_MULTIPART_DATA_FILE_CONTENTTYPE + && p.getValue().toLowerCase().contains("xml"))) + .map(NameValuePair::getName) + .collect(Collectors.toList()); + + for (NameValuePair originalPair : multipartVariant.getParamList()) { + if (xmlFileNames.contains(originalPair.getName()) + && (originalPair.getType() + == NameValuePair.TYPE_MULTIPART_DATA_FILE_PARAM)) { + scanForXxe(originalPair); + } + if (alertRaised) { + break; + } + } + } + } - // Check #1 : XXE Remote File Inclusion Attack - remoteFileInclusionAttack(); - - // Check #2 : Out-of-band XXE Attack - outOfBandFileInclusionAttack(); + private void scanForXxe(NameValuePair originalPair) { + remoteFileInclusionAttack(originalPair); - // Check if we've to do only basic analysis (only remote should be done)... - if (this.getAttackStrength() == AttackStrength.LOW) { - return; - } + outOfBandFileInclusionAttack(originalPair); - // Check #3 : XXE Local File Reflection Attack - localFileReflectionAttack(getNewMsg()); + if (this.getAttackStrength() == AttackStrength.LOW) { + return; + } - // Check if we've to do only medium sized analysis - // (only remote and reflected will be done) - if (this.getAttackStrength() == AttackStrength.MEDIUM) { - return; - } + localFileReflectionAttack(getNewMsg(), originalPair); - // Exit if the scan has been stopped - if (isStop()) { - return; - } + if (this.getAttackStrength() == AttackStrength.MEDIUM) { + return; + } - // Check #4 : XXE Local File Inclusion Attack - localFileInclusionAttack(getNewMsg()); + if (isStop()) { + return; } + + localFileInclusionAttack(getNewMsg(), originalPair); } /** @@ -229,7 +250,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 +267,12 @@ private void remoteFileInclusionAttack() { alert, XxeScanRule.class.getSimpleName()); String payload = MessageFormat.format(ATTACK_MESSAGE, callbackPayload); alert.setAttack(payload); - msg.setRequestBody(payload); + if (originalPair != null) { + multipartVariant.setParameter( + msg, originalPair, originalPair.getName(), payload); + } else { + msg.setRequestBody(payload); + } sendAndReceive(msg); } } catch (Exception e) { @@ -254,7 +280,7 @@ private void remoteFileInclusionAttack() { } } - private void outOfBandFileInclusionAttack() { + private void outOfBandFileInclusionAttack(NameValuePair originalPair) { try { ExtensionOast extOast = Control.getSingleton().getExtensionLoader().getExtension(ExtensionOast.class); @@ -269,12 +295,22 @@ 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) { + multipartVariant.setParameter( + msg, originalPair, 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) { + multipartVariant.setParameter( + msg, originalPair, originalPair.getName(), payload); + } else { + msg.setRequestBody(payload); + } sendAndReceive(msg); } } catch (Exception e) { @@ -282,29 +318,19 @@ private void outOfBandFileInclusionAttack() { } } - /** - * Local File Reflection Attack initially substitutes every attribute in the original XML - * request with a fake entity which includes a sensitive local file. The attack is repeated for - * every file listed in the LOCAL_FILE_TARGETS. The response returned is pattern matched against - * LOCAL_FILE_PATTERNS. An alert is raised when a match is found. If no alert is raised, then - * the process is repeated by replacing one attribute at a time, for a fixed number of - * attributes depending on the strength of the rule. - * - * @param msg new HttpMessage with the same request as the base. This is used to build the - * attack payload. - */ - 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,46 +344,37 @@ 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; } } } - /** - * Local File Inclusion Attack is described in - * https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing. The - * attack builds a payload for every file listed in LOCAL_FILE_TARGETS with the ATTACK_HEADER - * and the ATTACK_BODY. The response returned is pattern matched against LOCAL_FILE_PATTERNS. An - * alert is raised when a match is found. - * - *

This situation is very uncommon because it works only in case of a bare XML parser which - * execute the content and then returns the content almost untouched (maybe because it applies - * an XSLT or query it using XPath and give back the result) - * - * @param msg new HttpMessage with the same request as the base. This is used to build the - * attack payload. - */ - 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) { + multipartVariant.setParameter( + msg, originalPair, 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 +402,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) { + multipartVariant.setParameter(msg, originalPair, originalPair.getName(), payload); + } else { + msg.setRequestBody(payload); + } try { sendAndReceive(msg); } catch (IOException ex) { @@ -403,6 +425,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()) { From 409ef5db8f30940f27ef74f41b51174f5b326122 Mon Sep 17 00:00:00 2001 From: Cedric Buissart Date: Wed, 10 Jun 2026 17:21:31 +0000 Subject: [PATCH 2/2] refactor: use variant framework for multipart XXE detection Rework multipart handling to use AbstractAppParamPlugin's variant framework instead of manually creating VariantMultipartFormParameters. This ensures the user's input vector selection is respected. - Change XxeScanRule to extend AbstractAppParamPlugin - Override scan(List) to identify XML file parts by their declared content-type - Override scan(HttpMessage, NameValuePair) to run XXE attacks on matching multipart file parameters - Replace multipartVariant.setParameter() with the inherited setParameter() from AbstractAppParamPlugin - Restore Javadoc comments on scan(), localFileReflectionAttack(), and localFileInclusionAttack() Co-Authored-By: Claude Opus 4.6 --- .../zap/extension/ascanrules/XxeScanRule.java | 123 ++++++++++++------ 1 file changed, 85 insertions(+), 38 deletions(-) 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 664b4f64c22..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,20 +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 java.util.stream.Collectors; 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.core.scanner.VariantMultipartFormParameters; import org.parosproxy.paros.network.HttpMessage; import org.zaproxy.addon.commonlib.CommonAlertTag; import org.zaproxy.addon.commonlib.PolicyTag; @@ -50,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; @@ -132,7 +132,7 @@ public class XxeScanRule extends AbstractAppPlugin implements CommonActiveScanRu // Logger instance private static final Logger LOGGER = LogManager.getLogger(XxeScanRule.class); - private VariantMultipartFormParameters multipartVariant; + private Set xmlFileNames = Set.of(); private boolean alertRaised = false; @Override @@ -185,62 +185,86 @@ public int getRisk() { return Alert.RISK_HIGH; } + /** + * 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. + * + *

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() { - multipartVariant = null; alertRaised = false; HttpMessage msg = getBaseMsg(); String contentType = msg.getRequestHeader().getHeader(HttpFieldsNames.CONTENT_TYPE); if ((contentType != null) && (contentType.contains("xml"))) { + // Direct XML body — attack the whole body scanForXxe(null); - } else if ((contentType != null) && (contentType.contains("multipart"))) { - multipartVariant = new VariantMultipartFormParameters(); - multipartVariant.setMessage(getNewMsg()); - List xmlFileNames = - multipartVariant.getParamList().stream() - .filter( - p -> - (p.getType() - == NameValuePair - .TYPE_MULTIPART_DATA_FILE_CONTENTTYPE - && p.getValue().toLowerCase().contains("xml"))) - .map(NameValuePair::getName) - .collect(Collectors.toList()); - - for (NameValuePair originalPair : multipartVariant.getParamList()) { - if (xmlFileNames.contains(originalPair.getName()) - && (originalPair.getType() - == NameValuePair.TYPE_MULTIPART_DATA_FILE_PARAM)) { - scanForXxe(originalPair); - } - if (alertRaised) { - break; - } + } else { + // Let the framework iterate variants/parameters + super.scan(); + } + } + + @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); + } + + @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); + } + } + + @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. } private void scanForXxe(NameValuePair originalPair) { + // Check #1 : XXE Remote File Inclusion Attack remoteFileInclusionAttack(originalPair); + // Check #2 : Out-of-band XXE Attack outOfBandFileInclusionAttack(originalPair); + // 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 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); } @@ -268,8 +292,7 @@ private void remoteFileInclusionAttack(NameValuePair originalPair) { String payload = MessageFormat.format(ATTACK_MESSAGE, callbackPayload); alert.setAttack(payload); if (originalPair != null) { - multipartVariant.setParameter( - msg, originalPair, originalPair.getName(), payload); + setParameter(msg, originalPair.getName(), payload); } else { msg.setRequestBody(payload); } @@ -296,8 +319,7 @@ private void outOfBandFileInclusionAttack(NameValuePair originalPair) { String payload = MessageFormat.format(ATTACK_MESSAGE, "http://" + oastPayload); alert.setAttack(payload); if (originalPair != null) { - multipartVariant.setParameter( - msg, originalPair, originalPair.getName(), payload); + setParameter(msg, originalPair.getName(), payload); } else { msg.setRequestBody(payload); } @@ -306,8 +328,7 @@ private void outOfBandFileInclusionAttack(NameValuePair originalPair) { msg = getNewMsg(); payload = MessageFormat.format(ATTACK_MESSAGE, "https://" + oastPayload); if (originalPair != null) { - multipartVariant.setParameter( - msg, originalPair, originalPair.getName(), payload); + setParameter(msg, originalPair.getName(), payload); } else { msg.setRequestBody(payload); } @@ -318,6 +339,18 @@ private void outOfBandFileInclusionAttack(NameValuePair originalPair) { } } + /** + * Local File Reflection Attack initially substitutes every attribute in the original XML + * request with a fake entity which includes a sensitive local file. The attack is repeated for + * every file listed in the LOCAL_FILE_TARGETS. The response returned is pattern matched against + * LOCAL_FILE_PATTERNS. An alert is raised when a match is found. If no alert is raised, then + * the process is repeated by replacing one attribute at a time, for a fixed number of + * attributes depending on the strength of the rule. + * + * @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, NameValuePair originalPair) { String originalXml; if (originalPair != null) { @@ -350,6 +383,21 @@ private void localFileReflectionAttack(HttpMessage msg, NameValuePair originalPa } } + /** + * Local File Inclusion Attack is described in + * https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing. The + * attack builds a payload for every file listed in LOCAL_FILE_TARGETS with the ATTACK_HEADER + * and the ATTACK_BODY. The response returned is pattern matched against LOCAL_FILE_PATTERNS. An + * alert is raised when a match is found. + * + *

This situation is very uncommon because it works only in case of a bare XML parser which + * execute the content and then returns the content almost untouched (maybe because it applies + * an XSLT or query it using XPath and give back the result) + * + * @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, NameValuePair originalPair) { String payload = null; try { @@ -358,8 +406,7 @@ private void localFileInclusionAttack(HttpMessage msg, NameValuePair originalPai String localFile = LOCAL_FILE_TARGETS[idx]; payload = MessageFormat.format(ATTACK_MESSAGE, localFile); if (originalPair != null) { - multipartVariant.setParameter( - msg, originalPair, originalPair.getName(), payload); + setParameter(msg, originalPair.getName(), payload); } else { msg.setRequestBody(payload); } @@ -408,7 +455,7 @@ private boolean localFileReflectionTest(String requestBody, NameValuePair origin String localFile = LOCAL_FILE_TARGETS[idx]; String payload = MessageFormat.format(requestBody, localFile); if (originalPair != null) { - multipartVariant.setParameter(msg, originalPair, originalPair.getName(), payload); + setParameter(msg, originalPair.getName(), payload); } else { msg.setRequestBody(payload); }