AntiSamy is not working for special case

[satishraikwar]

Antisamy is not working for the test case , i tried in latest version also.
When there is "/" character inside tag it fails.

My Test Case:
@test
public void testXSSScript() throws PolicyException, ScanException {
String result = scanner.scan("<style/onload=alert(document.domain)>");
assertEquals("", result);
}

====Logic which called by test case===
Please consider policy is loading and i attached antisamy.xml , For some reason it is not giving any error for <style/onload=alert(document.domain)> when "Collection errors = r.getErrorMessages();" executes

public String scan(String untrustedUserInput) throws PolicyException, ScanException {
CleanResults r = webSecurityScanner.scan(untrustedUserInput, AntiSamy.SAX);
if(logger.isDebugEnabled()) {
logger.debug("Scanned request parameter in " + r.getScanTime() + "ms");
logger.debug("Value: " + untrustedUserInput);
logger.debug("Result: " + r.getCleanHTML());
logger.debug("Errors: " + r.getErrorMessages());
}

    Collection<String> errors = r.getErrorMessages();
    if(CollectionUtils.exists(errors, securityErrorPredicate)) {
        logger.info("Returning cleansed input due to " + errors.size() + " security errors: " + errors);
        logger.debug("Original: [" + untrustedUserInput + "]");

        final String cleansedHTML = fixMangledTags(r.getCleanHTML());
        logger.debug("Cleansed: [" + cleansedHTML + "]");
        return cleansedHTML;
    }
    return untrustedUserInput;
}

antisamy.zip

[davewichers]

What do you mean by it fails? It doesn't notice this at all? It strips it silently? It crashes?

What exactly do you want AntiSamy to do in this situation, as compared to what it does now?

[satishraikwar]

What do you mean by it fails? It doesn't notice this at all? It strips it silently? It crashes?

What exactly do you want AntiSamy to do in this situation, as compared to what it does now?

Can you check the test case which I have given.
<style/onload=alert(document.domain)> is not getting clean html. After Scan.

[nahsra]

I just added a GitHub Action that will automatically run `mvn test` after a push. Can you open a branch with this failing test case?

…

On Wed, Jun 10, 2020 at 9:19 PM satishraikwar ***@***.***> wrote: What do you mean by it fails? It doesn't notice this at all? It strips it silently? It crashes? What exactly do you want AntiSamy to do in this situation, as compared to what it does now? Can you check the test case which I have given. <style/onload=alert(document.domain)> is not getting clean html. After Scan. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAG6R6U6HG6R2LOUWTSFGMLRWAWL3ANCNFSM4N2FEAJA> .

[satishraikwar]

I just added a GitHub Action that will automatically run mvn test after a push. Can you open a branch with this failing test case?
…
On Wed, Jun 10, 2020 at 9:19 PM satishraikwar @.***> wrote: What do you mean by it fails? It doesn't notice this at all? It strips it silently? It crashes? What exactly do you want AntiSamy to do in this situation, as compared to what it does now? Can you check the test case which I have given. <style/onload=alert(document.domain)> is not getting clean html. After Scan. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG6R6U6HG6R2LOUWTSFGMLRWAWL3ANCNFSM4N2FEAJA .

Sure 👍

[satishraikwar]

Hi , i have written the test case which is cleaning the html but why it is giving the blank error message list ? Can you check the testcase. @test public void getErrorMessagesIssue() throws ScanException, PolicyException { Policy ebayPolicy = Policy.getInstance(getClass().getResource("/antisamy-ebay.xml")); String dirty = "<style/onload=alert(document.domain)>"; CleanResults cr = as.scan(dirty, ebayPolicy, AntiSamy.SAX); System.out.println("========================="+cr.getCleanHTML()); System.out.println("=============getErrorMessages============"+cr.getErrorMessages()); } Thanks Satish On Wed, Jun 10, 2020 at 9:25 PM Arshan Dabirsiaghi <notifications@github.com> wrote:

…

I just added a GitHub Action that will automatically run `mvn test` after a push. Can you open a branch with this failing test case? On Wed, Jun 10, 2020 at 9:19 PM satishraikwar ***@***.***> wrote: > What do you mean by it fails? It doesn't notice this at all? It strips it > silently? It crashes? > > What exactly do you want AntiSamy to do in this situation, as compared to > what it does now? > > Can you check the test case which I have given. > <style/onload=alert(document.domain)> is not getting clean html. After > Scan. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#44 (comment)>, or > unsubscribe > < https://github.com/notifications/unsubscribe-auth/AAG6R6U6HG6R2LOUWTSFGMLRWAWL3ANCNFSM4N2FEAJA > > . > — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALXL5MRUKCV4YLI5ZXHO6DLRWAXARANCNFSM4N2FEAJA> .

[davewichers]

OK. I created a new branch at: https://github.com/nahsra/antisamy/tree/issue44 which incorporates what it looks like @satishraikwar was trying to do. @nahsra - the results are a bit weird to me as well. I created 2 HTML strings (test44a & b) that incorporate the style of concern <style/onload=alert(document.domain)>.

In both cases, the resulting 'clean' HTML strips out this style (which is good I think). But in the first case an error message is created:
[The stylesheet code "</li> </ol> </body> </html>" could not be parsed.]

But in the second case, with simply a SPACE character after the style snippet, no error message is thrown. I also manually tried a much simpler HTML snippet and got similar results, except the error message was thrown in the snippet w/out the space instead of the one with the space.

@satishraikwar - given this test code, what are you expecting AntiSamy to do? I think in both cases it returns the correct HTML, but in only 1 case does it return an error message, which I do agree is probably not correct. And I think the error message is related to what's left in the document, not the <style > tag that was stripped out, even though what is left in the document is retained in the clean HTML.

[nahsra]

The getErrorMessages() API does not answer the question "is this safe input?" You must always use the sanitized input. The serialization and deserialization process is purposefully lossy and will filter attacks, but one of the tradeoffs is that we don't have an ability to see them in retrospect.

I am marking this as a dupe of issue #47.