jamesnetherton commented on code in PR #9116:
URL: https://github.com/apache/camel-quarkus/pull/9116#discussion_r3947180575
##########
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:
You were right to ask, and the test exposed it. After `reset()` the resolver
was gone and `document()` fetched and returned the external resource, so
`reset()` did discard the restriction.
Fixed by reinstalling it: `SecuredTransformer.reset()` now calls
`delegate.reset()` and puts the restricting resolver back. Chasing the same
idea turned up `setURIResolver(null)` as a second way to lose it, so a null now
falls back to the restriction rather than lifting it. The JDK is immune to both
because it enforces `ACCESS_EXTERNAL_STYLESHEET` through the factory rather
than through a `URIResolver`, so this was a parity gap either way.
That led to a third case in a follow-up commit.
`TransformerHandler.getTransformer()` and `TrAXFilter.getTransformer()` hand
out the transformer the handler then uses itself, so the same two calls took
the restriction off from underneath it. Both are now wrapped so
`getTransformer()` returns the secured transformer. Xalan reads its own
transformer from a field and never calls `getTransformer()`, so overriding it
does not disturb the transformation.
Tests: `documentFunctionStaysDeniedAfterTransformerReset`,
`documentFunctionStaysDeniedWhenUriResolverIsCleared`, and for the SAX paths
`documentFunctionStaysDeniedAfterResettingTheHandlersTransformer`,
`documentFunctionStaysDeniedWhenTheHandlersResolverIsCleared` and
`documentFunctionStaysDeniedAfterResettingTheFiltersTransformer`. Each one
fails without its fix. `transformerHandlerStillTransforms` and
`xmlFilterStillTransforms` pin that the new wrappers still delegate the SAX
events correctly.
The migration guide now states both semantics, since they differ from a
plain JAXP factory. Native mode is verified for the component path
(`xml-grouped` native ITs pass); the SAX entry points are covered by the JVM
tests only, as nothing in the repo drives that API.
--
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]
