andresriancho / owaspantisamy

Automatically exported from code.google.com/p/owaspantisamy
12 stars 15 forks source link

AntySamyDOMScanner doesn't report an error when I would expect it to. #190

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Copy of test xml rules, place in an appropriate src/test/java/resources 
directory:

<?xml version="1.0" encoding="ISO-8859-1"?>

    <!--
        W3C rules retrieved from:
        http://www.w3.org/TR/html401/struct/global.html
    -->

<anti-samy-rules xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:noNamespaceSchemaLocation="antisamy.xsd">

    <common-attributes>
        <attribute name="background">
            <regexp-list>
                <regexp value=".*" />
            </regexp-list>
        </attribute>
    </common-attributes>

    <tag-rules>
        <!-- Table tags -->
        <tag name="table" action="validate">
        </tag>
        <tag name="tbody" action="validate" />
        <tag name="tr" action="validate">
        </tag>
        <tag name="td" action="validate">
            <attribute name="background" />
        </tag>
    </tag-rules>
</anti-samy-rules>

Copy of Junit Test Case:
import static org.junit.Assert.assertEquals;

import org.junit.Before;
import org.junit.Test;
import org.owasp.validator.html.AntiSamy;
import org.owasp.validator.html.CleanResults;
import org.owasp.validator.html.Policy;

public class AntiSamyBugTest {

    private AntiSamy antiSamy;
    private Policy xssPolicy;

    @Before
    public void createAntiSamy() throws Exception {
        antiSamy = new AntiSamy();
        xssPolicy = Policy.getInstance(AntiSamyBugTest.class.getResource("/antisamy/antisamy-bug.xml"));
    }

    /**
     * This unit test deals explicitly with a discovered bug with the AntiSamy library where it is 
     * modifying user input before performing cleaning/scanning against the field in question.  
     * 
     * Once this is fixed, we should be able to adjust the expected results accordingly.  
     * 
     * @throws Exception
     */
    @Test
    public void testAntiSamyBug() throws Exception {
        String input = "<TABLE><TD BACKGROUND=\"<script>alert('xss')</script>\">Here Lies an Item.</TD></TABLE>";
        CleanResults cr = antiSamy.scan(input, xssPolicy);
        assertEquals(1, cr.getNumberOfErrors());
    }

}

Using Burp/ZAP we're able to alter request input to send in an input payload 
that looks like this.  While debugging into AntiSamy it appears that we're 
serializing input before our regex rules get applied, which has the effect of 
transforming the input before it gets checked by the regex portion.  (Unless 
I'm missing something, we don't check against our defined rules until line 193 
of AntiSamyDOMScanner, and the serialization occurs on 176.)

Original issue reported on code.google.com by xeno6...@gmail.com on 2 Apr 2015 at 1:13

GoogleCodeExporter commented 9 years ago
I want to add that I'm aware a more restrictive regex in the antisamy-bug.xml 
can alter this behavior "[A-Za-z0-9]*".  I don't think the issue here is the 
permissive nature of the regex, but I'm lacking a little background knowledge.  
In other cases where input gets nerfed or removed, we usually generate an 
error.  

Original comment by xeno6...@gmail.com on 2 Apr 2015 at 1:19