zhfeng commented on code in PR #9116:
URL: https://github.com/apache/camel-quarkus/pull/9116#discussion_r3939927179
##########
extensions-support/xalan/runtime/src/main/java/org/apache/camel/quarkus/support/xalan/XalanTransformerFactory.java:
##########
@@ -138,12 +198,224 @@ public TemplatesHandler newTemplatesHandler() throws
TransformerConfigurationExc
@Override
public XMLFilter newXMLFilter(Source source) throws
TransformerConfigurationException {
- return delegate.newXMLFilter(source);
+ return secure(delegate.newXMLFilter(source));
}
@Override
public XMLFilter newXMLFilter(Templates templates) throws
TransformerConfigurationException {
- return delegate.newXMLFilter(templates);
+ return secure(delegate.newXMLFilter(unwrap(templates)));
+ }
+
+ /**
+ * The SAX push entry points hand the caller a {@link Transformer} to
configure rather than one to call,
+ * and Xalan only copies the factory's {@link URIResolver} onto some of
them, so {@code document()} is
+ * restricted here for the same reason it is in {@link
#secure(Transformer)}. The document being
+ * transformed is parsed by the {@link XMLReader} the caller drives the
handler with, which is the
+ * caller's own choice just as a {@link SAXSource} carrying a reader is.
+ */
+ private TransformerHandler secure(TransformerHandler handler) {
+ handler.getTransformer().setURIResolver(restrictingUriResolver);
+ return handler;
+ }
+
+ /**
+ * {@link XMLFilter} has no accessor for the {@link Transformer} behind
it, so the restriction can only be
+ * installed on Xalan's own implementation. Guarded rather than cast
blindly so that a Xalan upgrade
+ * returning something else is reported instead of silently dropping the
restriction.
+ */
+ private XMLFilter secure(XMLFilter filter) {
+ if (filter instanceof TrAXFilter) {
+ ((TrAXFilter)
filter).getTransformer().setURIResolver(restrictingUriResolver);
+ } else {
+ LOGGER.warn("Expected an {} from the Xalan TransformerFactory but
got {}. The document() function"
+ + " may resolve external resources when transforming
through this XMLFilter.",
+ TrAXFilter.class.getName(), filter == null ? null :
filter.getClass().getName());
+ }
+ return filter;
+ }
+
+ /**
+ * Xalan casts {@link Templates} to its own {@code TemplatesImpl}
internally, so the wrapper has to be
+ * peeled off before handing one back to the delegate.
+ */
+ private static Templates unwrap(Templates templates) {
+ return templates instanceof SecuredTemplates ? ((SecuredTemplates)
templates).delegate : templates;
+ }
+
+ /**
+ * Parses {@code source} with a hardened {@link XMLReader} unless it has
already been parsed, or the
+ * caller supplied its own reader. Mirrors what {@code
XmlConverter.createSAXParserFactory()} does for
+ * the bodies camel-xslt converts itself, so that {@link Source}-shaped
bodies get the same treatment.
+ */
+ private static Source secureInputSource(Source source) throws
TransformerException {
+ if (source instanceof StreamSource) {
+ final StreamSource streamSource = (StreamSource) source;
+ final InputStream inputStream = streamSource.getInputStream();
+ final Reader reader = streamSource.getReader();
+ if (inputStream == null && reader == null) {
+ // Nothing but a systemId; let the delegate resolve it as
before
+ return source;
+ }
+ final InputSource inputSource = new InputSource();
+ inputSource.setSystemId(streamSource.getSystemId());
+ if (inputStream != null) {
+ inputSource.setByteStream(inputStream);
+ } else {
+ inputSource.setCharacterStream(reader);
+ }
+ return new SAXSource(createSecureXmlReader(), inputSource);
+ }
+ if (source instanceof SAXSource) {
+ final SAXSource saxSource = (SAXSource) source;
+ if (saxSource.getXMLReader() == null) {
+ return new SAXSource(createSecureXmlReader(),
saxSource.getInputSource());
+ }
+ }
+ // DOMSource and StAXSource are already parsed; a caller supplied
XMLReader is the caller's own choice
+ return source;
}
+ private static XMLReader createSecureXmlReader() throws
TransformerException {
+ final SAXParserFactory factory = SAXParserFactory.newInstance();
+ factory.setNamespaceAware(true);
+ setFeature(factory, javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING,
true);
+ setFeature(factory, EXTERNAL_GENERAL_ENTITIES, false);
+ try {
+ return factory.newSAXParser().getXMLReader();
+ } catch (ParserConfigurationException | SAXException e) {
+ throw new TransformerException("Could not create a secure
XMLReader for the input document", e);
+ }
+ }
+
+ private static void setFeature(SAXParserFactory factory, String name,
boolean value) {
+ try {
+ factory.setFeature(name, value);
+ } catch (ParserConfigurationException | SAXException e) {
+ LOGGER.warn("SAXParserFactory does not support the feature {} with
value {}, due to {}."
+ + " External entities in XSLT input documents may be
resolved.", name, value, e.getMessage());
+ }
+ }
+
+ /**
+ * Denies external references that the application's own {@link
URIResolver} did not resolve. This is the
+ * {@link XMLConstants#ACCESS_EXTERNAL_STYLESHEET} behaviour the JDK
applies when
+ * {@link XMLConstants#FEATURE_SECURE_PROCESSING} is enabled, which Xalan
does not implement. It takes
+ * effect for {@code document()} at transform time; see the class javadoc
for why compile time
+ * {@code xsl:import}/{@code xsl:include} cannot be covered.
+ */
+ private final class RestrictingUriResolver implements URIResolver {
+ @Override
+ public Source resolve(String href, String base) throws
TransformerException {
+ final URIResolver resolver = applicationUriResolver;
+ if (resolver != null) {
+ final Source source = resolver.resolve(href, base);
+ if (source != null) {
+ return source;
+ }
+ }
+ throw new TransformerException(
+ "Access to the external resource '" + href + "' (base '" +
base + "') is not allowed."
+ + " Resolve it through a
javax.xml.transform.URIResolver if it is required.");
+ }
+ }
+
+ /**
+ * Ensures {@link SecuredTransformer} is used for transformers obtained
from compiled templates, which is
+ * how camel-xslt gets hold of them.
+ */
+ private static final class SecuredTemplates implements Templates {
+ private final Templates delegate;
+ private final XalanTransformerFactory factory;
+
+ SecuredTemplates(Templates delegate, XalanTransformerFactory factory) {
+ this.delegate = delegate;
+ this.factory = factory;
+ }
+
+ @Override
+ public Transformer newTransformer() throws
TransformerConfigurationException {
+ return factory.secure(delegate.newTransformer());
+ }
+
+ @Override
+ public Properties getOutputProperties() {
+ return delegate.getOutputProperties();
+ }
+ }
+
+ /**
+ * Applies {@link XalanTransformerFactory#secureInputSource(Source)} to
the document being transformed.
+ */
+ private static final class SecuredTransformer extends Transformer {
+ private final Transformer delegate;
+
+ SecuredTransformer(Transformer delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void transform(Source xmlSource, javax.xml.transform.Result
outputTarget) throws TransformerException {
+ delegate.transform(secureInputSource(xmlSource), outputTarget);
+ }
+
+ @Override
+ public void setParameter(String name, Object value) {
+ delegate.setParameter(name, value);
+ }
+
+ @Override
+ public Object getParameter(String name) {
+ return delegate.getParameter(name);
+ }
+
+ @Override
+ public void clearParameters() {
+ delegate.clearParameters();
+ }
+
+ @Override
+ public void setURIResolver(URIResolver resolver) {
+ delegate.setURIResolver(resolver);
+ }
+
+ @Override
+ public URIResolver getURIResolver() {
+ return delegate.getURIResolver();
+ }
+
+ @Override
+ public void setOutputProperties(Properties oformat) {
+ delegate.setOutputProperties(oformat);
+ }
+
+ @Override
+ public Properties getOutputProperties() {
+ return delegate.getOutputProperties();
+ }
+
+ @Override
+ public void setOutputProperty(String name, String value) {
+ delegate.setOutputProperty(name, value);
+ }
+
+ @Override
+ public String getOutputProperty(String name) {
+ return delegate.getOutputProperty(name);
+ }
+
+ @Override
+ public void setErrorListener(ErrorListener listener) {
+ delegate.setErrorListener(listener);
+ }
+
+ @Override
+ public ErrorListener getErrorListener() {
+ return delegate.getErrorListener();
+ }
+
+ @Override
+ public void reset() {
+ delegate.reset();
Review Comment:
Could we add a regression test for the behavior after `reset()`? The
restricting URI resolver is installed on the delegate after the Xalan
transformer is created, while JAXP defines `reset()` as restoring the
transformer's original configuration. It would be useful to verify that calling
`reset()` here does not discard `RestrictingUriResolver` and allow a subsequent
transformation using `document()` to resolve an external resource. If the test
exposes that behavior, this override could reinstall the restricting resolver
after `delegate.reset()`.
--
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]
