From 10625d1a4488ddfd8dd2af4f30a7162772357d68 Mon Sep 17 00:00:00 2001 From: Logic Date: Thu, 22 Jan 2026 17:07:47 +0800 Subject: [PATCH 1/3] [Refactor] tighten resource constraints for XML/XPath collector --- .../collect/http/HttpCollectImpl.java | 91 ++++++++++++++++++- .../constants/CollectorConstants.java | 24 +++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java b/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java index 2bd9132d226..745723299a9 100644 --- a/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java +++ b/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java @@ -39,6 +39,7 @@ import java.util.Set; import java.util.stream.Collectors; import javax.net.ssl.SSLException; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.xpath.XPath; @@ -100,12 +101,29 @@ import java.util.HashSet; import java.util.ArrayList; import java.util.Collections; +import java.util.List; /** * http https collect */ @Slf4j public class HttpCollectImpl extends AbstractCollect { + + /** + * Pre-compiled regex patterns for dangerous XPath detection. + * Compiled once at class load for performance. + */ + private static final List DANGEROUS_XPATH_PATTERNS; + + static { + List patterns = new ArrayList<>(); + for (String pattern : CollectorConstants.DANGEROUS_XPATH_PATTERNS) { + // Use CASE_INSENSITIVE to prevent bypass via case variations (e.g., //TEXT() vs //text()) + patterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE | Pattern.DOTALL)); + } + DANGEROUS_XPATH_PATTERNS = Collections.unmodifiableList(patterns); + } + private final Set defaultSuccessStatusCodes = Set.of( HttpStatus.SC_OK, HttpStatus.SC_CREATED, @@ -357,21 +375,83 @@ private void parseResponseBySiteMap(String resp, List aliasFields, } } + /** + * Validates the XPath expression to prevent DoS attacks. + * Checks for dangerous patterns that could traverse the entire XML document. + * Uses pre-compiled patterns for performance and case-insensitive matching for security. + * + * @param xpathExpression the XPath expression to validate + * @throws IllegalArgumentException if the expression contains dangerous patterns + */ + private void validateXPathExpression(String xpathExpression) throws IllegalArgumentException { + if (!StringUtils.hasText(xpathExpression)) { + return; + } + + // Check against dangerous patterns using pre-compiled regex + for (Pattern pattern : DANGEROUS_XPATH_PATTERNS) { + Matcher matcher = pattern.matcher(xpathExpression); + if (matcher.find()) { + throw new IllegalArgumentException( + "XPath expression contains dangerous pattern that may cause DoS: " + pattern.pattern() + ); + } + } + + // Check for excessive wildcard usage (more than 3 // or * operators) + long descendantAxisCount = xpathExpression.chars().filter(ch -> ch == '/').count(); + long wildcardCount = xpathExpression.chars().filter(ch -> ch == '*').count(); + + if (descendantAxisCount > 10 || wildcardCount > 5) { + throw new IllegalArgumentException( + "XPath expression contains too many wildcards or descendant axes, potential DoS risk" + ); + } + + log.debug("XPath expression validation passed: {}", xpathExpression); + } + private void parseResponseByXmlPath(String resp, Metrics metrics, CollectRep.MetricsData.Builder builder, Long responseTime) { HttpProtocol http = metrics.getHttp(); List aliasFields = metrics.getAliasFields(); String xpathExpression = http.getParseScript(); + + // Layer 1: Validate XPath expression is not empty if (!StringUtils.hasText(xpathExpression)) { log.warn("Http collect parse type is xmlPath, but the xpath expression is empty."); builder.setCode(CollectRep.Code.FAIL); builder.setMsg("XPath expression is empty"); return; } + + // Layer 2: Check XML response size to prevent memory exhaustion + if (resp != null && resp.length() > CollectorConstants.MAX_XML_RESPONSE_SIZE) { + log.warn("XML response size {} bytes exceeds maximum allowed size {} bytes", + resp.length(), CollectorConstants.MAX_XML_RESPONSE_SIZE); + builder.setCode(CollectRep.Code.FAIL); + builder.setMsg("XML response exceeds maximum allowed size of " + + (CollectorConstants.MAX_XML_RESPONSE_SIZE / 1024 / 1024) + "MB"); + return; + } + + // Layer 3: Validate XPath expression for dangerous patterns + try { + validateXPathExpression(xpathExpression); + } catch (IllegalArgumentException e) { + log.warn("XPath expression validation failed: {}", e.getMessage()); + builder.setCode(CollectRep.Code.FAIL); + builder.setMsg(e.getMessage()); + return; + } + int keywordNum = CollectUtil.countMatchKeyword(resp, http.getKeyword()); try { DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + + // Layer 4: Enable XML secure processing and XXE protection + dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); @@ -408,7 +488,16 @@ private void parseResponseByXmlPath(String resp, Metrics metrics, return; } - for (int i = 0; i < nodeList.getLength(); i++) { + // Layer 5: Limit the number of results to prevent excessive resource consumption + int resultSize = nodeList.getLength(); + int maxResults = CollectorConstants.MAX_XPATH_RESULT_NODES; + if (resultSize > maxResults) { + log.warn("XPath expression returned {} nodes, exceeding limit of {}. Processing first {} nodes only.", + resultSize, maxResults, maxResults); + resultSize = maxResults; + } + + for (int i = 0; i < resultSize; i++) { Node node = nodeList.item(i); CollectRep.ValueRow.Builder valueRowBuilder = CollectRep.ValueRow.newBuilder(); diff --git a/hertzbeat-collector/hertzbeat-collector-common/src/main/java/org/apache/hertzbeat/collector/constants/CollectorConstants.java b/hertzbeat-collector/hertzbeat-collector-common/src/main/java/org/apache/hertzbeat/collector/constants/CollectorConstants.java index a52092e2f0e..4536e362061 100644 --- a/hertzbeat-collector/hertzbeat-collector-common/src/main/java/org/apache/hertzbeat/collector/constants/CollectorConstants.java +++ b/hertzbeat-collector/hertzbeat-collector-common/src/main/java/org/apache/hertzbeat/collector/constants/CollectorConstants.java @@ -49,4 +49,28 @@ public interface CollectorConstants extends NetworkConstants { String STATUS_CODE = "statusCode"; + /** + * Maximum XML response size in bytes (10MB) to prevent DoS attacks + */ + int MAX_XML_RESPONSE_SIZE = 10 * 1024 * 1024; + + /** + * Maximum number of nodes returned by XPath query to prevent excessive resource consumption + */ + int MAX_XPATH_RESULT_NODES = 1000; + + /** + * Dangerous XPath expression patterns that could cause DoS attacks + * These patterns match expressions that traverse the entire XML document + */ + String[] DANGEROUS_XPATH_PATTERNS = { + "//\\*\\s*\\|\\s*//@\\*\\s*\\|\\s*//text\\(\\)", // //* | //@* | //text() + "//\\*\\s*\\|", // //* | ... + "//@\\*\\s*\\|", // //@* | ... + "//node\\(\\)\\s*\\|", // //node() | ... + "descendant-or-self::node\\(\\)\\s*\\|", // descendant-or-self::node() | ... + "/descendant-or-self::node\\(\\)", // /descendant-or-self::node() + "//\\*[\\s\\S]*//\\*" // //** with multiple wildcards + }; + } \ No newline at end of file From d9403ec3004717d7b2e69e132ef796554917fd6f Mon Sep 17 00:00:00 2001 From: Logic Date: Thu, 22 Jan 2026 17:08:58 +0800 Subject: [PATCH 2/3] [Refactor] tighten resource constraints for XML/XPath collector --- .../hertzbeat/collector/collect/http/HttpCollectImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java b/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java index 745723299a9..13709828182 100644 --- a/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java +++ b/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java @@ -101,7 +101,6 @@ import java.util.HashSet; import java.util.ArrayList; import java.util.Collections; -import java.util.List; /** * http https collect @@ -971,4 +970,4 @@ private boolean checkSuccessInvoke(Metrics metrics, int statusCode) { } return successCodeSet.contains(statusCode); } -} \ No newline at end of file +} From cb54b52cf9cc09aae2f70712fa302f40bdd23fcc Mon Sep 17 00:00:00 2001 From: Logic Date: Thu, 22 Jan 2026 17:18:43 +0800 Subject: [PATCH 3/3] [Refactor] tighten resource constraints for XML/XPath collector --- .../collect/http/HttpCollectImpl.java | 32 +++++++++---------- .../constants/CollectorConstants.java | 4 +-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java b/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java index 13709828182..03e2d0dc4c5 100644 --- a/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java +++ b/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java @@ -109,7 +109,7 @@ public class HttpCollectImpl extends AbstractCollect { /** - * Pre-compiled regex patterns for dangerous XPath detection. + * Pre-compiled regex patterns for dangerous Xpath detection. * Compiled once at class load for performance. */ private static final List DANGEROUS_XPATH_PATTERNS; @@ -375,14 +375,14 @@ private void parseResponseBySiteMap(String resp, List aliasFields, } /** - * Validates the XPath expression to prevent DoS attacks. + * Validates the Xpath expression to prevent DoS attacks. * Checks for dangerous patterns that could traverse the entire XML document. * Uses pre-compiled patterns for performance and case-insensitive matching for security. * - * @param xpathExpression the XPath expression to validate + * @param xpathExpression the Xpath expression to validate * @throws IllegalArgumentException if the expression contains dangerous patterns */ - private void validateXPathExpression(String xpathExpression) throws IllegalArgumentException { + private void validateXpathExpression(String xpathExpression) throws IllegalArgumentException { if (!StringUtils.hasText(xpathExpression)) { return; } @@ -392,7 +392,7 @@ private void validateXPathExpression(String xpathExpression) throws IllegalArgum Matcher matcher = pattern.matcher(xpathExpression); if (matcher.find()) { throw new IllegalArgumentException( - "XPath expression contains dangerous pattern that may cause DoS: " + pattern.pattern() + "Xpath expression contains dangerous pattern that may cause DoS: " + pattern.pattern() ); } } @@ -403,11 +403,11 @@ private void validateXPathExpression(String xpathExpression) throws IllegalArgum if (descendantAxisCount > 10 || wildcardCount > 5) { throw new IllegalArgumentException( - "XPath expression contains too many wildcards or descendant axes, potential DoS risk" + "Xpath expression contains too many wildcards or descendant axes, potential DoS risk" ); } - log.debug("XPath expression validation passed: {}", xpathExpression); + log.debug("Xpath expression validation passed: {}", xpathExpression); } private void parseResponseByXmlPath(String resp, Metrics metrics, @@ -416,11 +416,11 @@ private void parseResponseByXmlPath(String resp, Metrics metrics, List aliasFields = metrics.getAliasFields(); String xpathExpression = http.getParseScript(); - // Layer 1: Validate XPath expression is not empty + // Layer 1: Validate Xpath expression is not empty if (!StringUtils.hasText(xpathExpression)) { log.warn("Http collect parse type is xmlPath, but the xpath expression is empty."); builder.setCode(CollectRep.Code.FAIL); - builder.setMsg("XPath expression is empty"); + builder.setMsg("Xpath expression is empty"); return; } @@ -434,11 +434,11 @@ private void parseResponseByXmlPath(String resp, Metrics metrics, return; } - // Layer 3: Validate XPath expression for dangerous patterns + // Layer 3: Validate Xpath expression for dangerous patterns try { - validateXPathExpression(xpathExpression); + validateXpathExpression(xpathExpression); } catch (IllegalArgumentException e) { - log.warn("XPath expression validation failed: {}", e.getMessage()); + log.warn("Xpath expression validation failed: {}", e.getMessage()); builder.setCode(CollectRep.Code.FAIL); builder.setMsg(e.getMessage()); return; @@ -466,7 +466,7 @@ private void parseResponseByXmlPath(String resp, Metrics metrics, NodeList nodeList = (NodeList) xpath.evaluate(xpathExpression, document, XPathConstants.NODESET); if (nodeList == null || nodeList.getLength() == 0) { - log.debug("XPath expression '{}' returned no nodes.", xpathExpression); + log.debug("Xpath expression '{}' returned no nodes.", xpathExpression); boolean requestedSummaryFields = aliasFields.stream() .anyMatch(alias -> NetworkConstants.RESPONSE_TIME.equalsIgnoreCase(alias) || CollectorConstants.KEYWORD.equalsIgnoreCase(alias)); @@ -491,7 +491,7 @@ private void parseResponseByXmlPath(String resp, Metrics metrics, int resultSize = nodeList.getLength(); int maxResults = CollectorConstants.MAX_XPATH_RESULT_NODES; if (resultSize > maxResults) { - log.warn("XPath expression returned {} nodes, exceeding limit of {}. Processing first {} nodes only.", + log.warn("Xpath expression returned {} nodes, exceeding limit of {}. Processing first {} nodes only.", resultSize, maxResults, maxResults); resultSize = maxResults; } @@ -510,7 +510,7 @@ private void parseResponseByXmlPath(String resp, Metrics metrics, String value = (String) xpath.evaluate(alias, node, XPathConstants.STRING); valueRowBuilder.addColumn(StringUtils.hasText(value) ? value : CommonConstants.NULL_VALUE); } catch (XPathExpressionException e) { - log.warn("Failed to evaluate XPath '{}' for node [{}]: {}", alias, node.getNodeName(), e.getMessage()); + log.warn("Failed to evaluate Xpath '{}' for node [{}]: {}", alias, node.getNodeName(), e.getMessage()); valueRowBuilder.addColumn(CommonConstants.NULL_VALUE); } } @@ -519,7 +519,7 @@ private void parseResponseByXmlPath(String resp, Metrics metrics, } } catch (Exception e) { - log.warn("Failed to parse XML response with XPath '{}': {}", xpathExpression, e.getMessage(), e); + log.warn("Failed to parse XML response with Xpath '{}': {}", xpathExpression, e.getMessage(), e); builder.setCode(CollectRep.Code.FAIL); builder.setMsg("Failed to parse XML response: " + e.getMessage()); } diff --git a/hertzbeat-collector/hertzbeat-collector-common/src/main/java/org/apache/hertzbeat/collector/constants/CollectorConstants.java b/hertzbeat-collector/hertzbeat-collector-common/src/main/java/org/apache/hertzbeat/collector/constants/CollectorConstants.java index 4536e362061..32d7b29de3e 100644 --- a/hertzbeat-collector/hertzbeat-collector-common/src/main/java/org/apache/hertzbeat/collector/constants/CollectorConstants.java +++ b/hertzbeat-collector/hertzbeat-collector-common/src/main/java/org/apache/hertzbeat/collector/constants/CollectorConstants.java @@ -55,12 +55,12 @@ public interface CollectorConstants extends NetworkConstants { int MAX_XML_RESPONSE_SIZE = 10 * 1024 * 1024; /** - * Maximum number of nodes returned by XPath query to prevent excessive resource consumption + * Maximum number of nodes returned by Xpath query to prevent excessive resource consumption */ int MAX_XPATH_RESULT_NODES = 1000; /** - * Dangerous XPath expression patterns that could cause DoS attacks + * Dangerous Xpath expression patterns that could cause DoS attacks * These patterns match expressions that traverse the entire XML document */ String[] DANGEROUS_XPATH_PATTERNS = {