ppkarwasz opened a new issue, #4401: URL: https://github.com/apache/hertzbeat/issues/4401
### Feature Request Obtain the JAXP factories in `HttpCollectImpl` from [Apache Commons Secure XML](https://commons.apache.org/proper/commons-secure-xml/) instead of configuring security features by hand. ### Is your feature request related to a problem? Please describe `HttpCollectImpl` parses XML returned by monitored endpoints, which is untrusted input, in two places, and each configures its own subset of security features: - [the sitemap parser](https://github.com/apache/hertzbeat/blob/7177bb4a7/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java#L332-L335) only sets `disallow-doctype-decl` and disables XInclude; - [the XPath parser](https://github.com/apache/hertzbeat/blob/7177bb4a7/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java#L487-L500) sets six features, while its `XPathFactory` is left unconfigured. Neither is wrong today, but this kind of per-call-site configuration tends to drift: every new parser has to remember the full list, and whether a feature is honored depends on the JAXP implementation on the classpath. ### Describe the solution you'd like Replace `DocumentBuilderFactory.newInstance()` and `XPathFactory.newInstance()` with `SecureDocumentBuilderFactory.newInstance()` and `SecureXPathFactory.newInstance()`, and drop the manual feature settings. Commons Secure XML installs non-removable resolvers that ignore external resources (DTDs, external entities, XInclude), and it fails loudly if the underlying implementation cannot be secured. Its guarantees are documented in the [threat model](https://commons.apache.org/proper/commons-secure-xml/threat_model.html) and tested across the JDK, Xerces, Woodstox and Saxon. To keep raw factories from coming back, a [forbidden-apis](https://github.com/policeman-tools/forbidden-apis) check can reject the plain JAXP factory methods in main code. It needs no XXE unit tests in Hertzbeat, since those would only re-test the library. The only behavior change: a response containing a DOCTYPE is no longer rejected; it is parsed, and any external references in it are ignored. ### Describe alternatives you've considered Keeping the handwritten configuration and aligning the two call sites. It works, but it leaves the protection dependent on each call site and on the parser implementation. ### Additional context The library is Java 8+, has no runtime dependencies and is licensed under Apache-2.0. I have a PR ready. Unrelated finding: [`XmlUtil.fromXml(String, TypeReference)`](https://github.com/apache/hertzbeat/blob/7177bb4a7/hertzbeat-common-core/src/main/java/org/apache/hertzbeat/common/util/XmlUtil.java) checks `!StringUtils.isEmpty(xml)` instead of `StringUtils.isEmpty(xml)`, so it returns `null` for every non-empty input. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
