Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identified a checkmarx vulnerability #536

Closed
Purnaank opened this issue Nov 21, 2024 · 4 comments
Closed

Identified a checkmarx vulnerability #536

Purnaank opened this issue Nov 21, 2024 · 4 comments

Comments

@Purnaank
Copy link

Purnaank commented Nov 21, 2024

We are using custom antisamy library, which overrides some features of this library. However we have kept most of the classes and its functionality same as the actual library.

On running a checkmarx scan on it, a MEDIUM level vulnerability has been identified. Below is the description of the same.

             **Source**	                                                        **Destination**

File src/main/java/org/owasp/validator/html/
scan/AntiSamySAXScanner.java src/main/java/org/owasp/validator/html/
scan/AntiSamySAXScanner.java

Line 81 237

Object getProperty transform

Improper Restriction of Stored XXE Ref\Path 1:
The scan loads and parses XML using transform, at line 206 of
src/main/java/org/owasp/validator/html/scan/AntiSamySAXScanner.java. This XML was
constructed based on stored data, getProperty, which was read from the database or a file at
line 69 of src/main/java/org/owasp/validator/html/scan/AntiSamySAXScanner.java. Note that
transform is set to automatically load and replace any DTD entity references in the XML,
including references to external files.

Screenshot 2024-11-21 121600

We would appreciate your feedback and inputs on the same.

We are planning to remove the flexibility to provide the implementation of TransformerFactory at startup using System.getProperty(). Instead we will keep it hard-coded and will force to use the same default Implementation (com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl) every time.

@davewichers
Copy link
Collaborator

There is one similar warning from SpotBugs in AntiSamy, but since the method is private and AntiSamy itself is providing the XML as a bundled resource, we trust the XML, so we are not concerned. Where is your XML coming from? Do you trust it? If not, you might have to tweak the code with the issue to prevent XXE, assuming that tweak doesn't break how you are using the XML.

@Purnaank
Copy link
Author

Thank you for the response.
The incoming XML is from an un-trusted source.
We will tweak the code accordingly to prevent XXE.

@kwwall
Copy link
Contributor

kwwall commented Nov 27, 2024

@davewichers - I think you should close this issue, as no fix is necessary to AntiSamy since this was a custom implementation.

@davewichers
Copy link
Collaborator

Closing this issue as it's a conversation about a customized fork of AntiSamy, not the version we maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants