Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| DocumentBuilder builder = builderFactory.newDocumentBuilder(); | ||
| Document inputDoc = builder.parse(contentStream); | ||
| transformer.transform(new DOMSource(inputDoc), new StreamResult(writer)); | ||
| transformer.transform(new StreamSource(contentStream), new StreamResult(writer)); |
Check failure
Code scanning / CodeQL
Resolving XML external entity in user-controlled data Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To mitigate XXE attacks, you should ensure the TransformerFactory used for the XSLT template is securely configured. The safest approach is to instantiate your own TransformerFactory, configure it to disable external entity access and DTDs, load the template using this factory, and then create the Transformer. Specifically, before calling newTransformer(), set the following:
XMLConstants.FEATURE_SECURE_PROCESSING: enabled.XMLConstants.ACCESS_EXTERNAL_DTD: empty string.XMLConstants.ACCESS_EXTERNAL_STYLESHEET: empty string.
These settings should be applied immediately before the transformation on line 170.
Modify the block starting at line 162–165 to instantiate and configure a TransformerFactory securely, load the template, and use the Transformer for the transformation.
What to change:
- At the location you use
profilesService.getTemplate(profile.getMetadataxslt()), instantiate and configure a secureTransformerFactory, use it to load the template from the XSLT, and create theTransformerinstance. - You may need a method to load a
Templatesobject from an XSLT string using a secureTransformerFactory.
| @@ -160,9 +160,12 @@ | ||
| } | ||
|
|
||
| // create transformer | ||
| Templates template = profilesService.getTemplate(profile.getMetadataxslt()); | ||
| TransformerFactory factory = TransformerFactory.newInstance(); | ||
| factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); | ||
| factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); | ||
| factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); | ||
| Templates template = factory.newTemplates(new StreamSource(new ByteArrayInputStream(profile.getMetadataxslt().getBytes("UTF-8")))); | ||
| Transformer transformer = template.newTransformer(); | ||
|
|
||
| try (ByteArrayInputStream contentStream = new ByteArrayInputStream(response.getBytes("UTF-8"));) { | ||
|
|
||
| // perform transformation |
| Document inputDoc = builder.parse(contentStream); | ||
|
|
||
| transformer.transform(new DOMSource(inputDoc), new StreamResult(writer)); | ||
| transformer.transform(new StreamSource(contentStream), new StreamResult(writer)); |
Check failure
Code scanning / CodeQL
Resolving XML external entity in user-controlled data Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix this XXE vulnerability, we must ensure that the XML processor used for transforming untrusted data (specifically, the TransformerFactory from which Templates/Transformer are created) is securely configured so that:
- Document type declarations are disallowed.
- External entity processing is disabled.
- References to external DTDs must not be resolved.
The secure flags should be set on the TransformerFactory both before compiling the XSLT and before executing transformations. In practice, if profilesService.getTemplate() wraps that logic, its implementation must do this. However, since we can't alter code outside the provided code snippet, we must ensure that the TransformerFactory used in this file is configured securely, especially before calling getTemplate(profile.getResponsexslt()).
If the code controls Templates allocation with a local TransformerFactory (not shown in the snippet), we can demonstrate how to safely instantiate one and use it for transformations in place. Otherwise, we'd need to set properties globally on the default TransformerFactory, or, if only provided with a local reference to the loaded Templates, we would need to verify its source.
Assuming profilesService.getTemplate() is what produces Templates, and if we can't edit that method, the best remediation in the provided code is to securely instantiate the TransformerFactory before using it, then compile the Templates securely, and securely transform user XML with this factory.
If necessary, in the block that does the transformation (lines 304-312), create a securely configured TransformerFactory, use that to produce Templates and Transformer, and apply the transformation.
Required imports: none, as all pertinent classes are already imported.
Therefore, in method readRecords, replace the existing transformation block to:
- Securely instantiate a
TransformerFactoryand set features to disable external entities. - Use this factory to create the
Transformer(andTemplatesif needed) for the transformation.
| @@ -301,8 +301,20 @@ | ||
| private List<IRecord> readRecords(InputStream contentStream) throws IOException, TransformerConfigurationException, TransformerException, ParserConfigurationException, SAXException, XPathExpressionException { | ||
| ArrayList<IRecord> records = new ArrayList<>(); | ||
|
|
||
| // create transformer | ||
| Templates template = profilesService.getTemplate(profile.getResponsexslt()); | ||
| // create transformer with XXE protection | ||
| TransformerFactory tfactory = TransformerFactory.newInstance(); | ||
| tfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); | ||
| try { | ||
| tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); | ||
| } catch (IllegalArgumentException ex) { | ||
| LOG.warn("TransformerFactory does not support ACCESS_EXTERNAL_DTD: " + ex.getMessage()); | ||
| } | ||
| try { | ||
| tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); | ||
| } catch (IllegalArgumentException ex) { | ||
| LOG.warn("TransformerFactory does not support ACCESS_EXTERNAL_STYLESHEET: " + ex.getMessage()); | ||
| } | ||
| Templates template = tfactory.newTemplates(new StreamSource(profilesService.getTemplate(profile.getResponsexslt()).getTransformer().getURI())); | ||
| Transformer transformer = template.newTransformer(); | ||
|
|
||
| // perform transformation |
2 participants
To resolved CSW harvesting issue