rjatkins / owaspantisamy

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

Seemingly-equivalent attribute tags yield two different behaviors #8

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. On the following HTML:

<b id="feh" fehOnly="feh" intOnly="123d" badAtt="poison">woot</b>

2. Use the following filter:

        <tag name="b" action="validate">
            <attribute name="id">
            </attribute>
            <attribute name="fehonly">
                <literal-list>
                    <literal value="feh"/>
                </literal-list>
            </attribute>
            <attribute name="intonly">
                <regexp-list>
                    <regexp name="integer"/>
                </regexp-list>
            </attribute>
        </tag>

3. And you will get:

***** DIRTY HTML *****
<b id="feh" fehOnly="feh" intOnly="123d" badAtt="poison">woot</b>
***** CLEAN HTML *****
<b fehonly="feh">woot</b>

***** ERRORS *****
The <b>badatt</b> attribute of the <b>b</b> tag has been removed for
security reasons. This removal should not affect the display of the HTML
submitted.
The <b>b</b> tag contained an attribute that we couldn't process. The
<b>id</b> attribute had a value of <u>feh</u>. This value could not be
accepted for security reasons. We have chosen to remove the <b>id</b>
attribute from the tag and leave everything else in place so that we could
process this input.
The <b>b</b> tag contained an attribute that we couldn't process. The
<b>intonly</b> attribute had a value of <u>123d</u>. This value could not
be accepted for security reasons. We have chosen to remove the
<b>intonly</b> attribute from the tag and leave everything else in place so
that we could process this input.

What is the expected output? What do you see instead?

When the following (a):

            <attribute name="id">
            </attribute>

Is changed to (b):

            <attribute name="id"></attribute>

Or even (c):

            <attribute name="id"/>

The output is instead:

***** DIRTY HTML *****
<b id="feh" fehOnly="feh" intOnly="123d" badAtt="poison">woot</b>
***** CLEAN HTML *****
<b fehonly="feh" id="feh">woot</b>

***** ERRORS *****
The <b>badatt</b> attribute of the <b>b</b> tag has been removed for
security reasons. This removal should not affect the display of the HTML
submitted.
The <b>b</b> tag contained an attribute that we couldn't process. The
<b>intonly</b> attribute had a value of <u>123d</u>. This value could not
be accepted for security reasons. We have chosen to remove the
<b>intonly</b> attribute from the tag and leave everything else in place so
that we could process this input.

Notice that the *id* attribute is preserved.

What version of the product are you using? On what operating system?
AntiSamy v.1.1.1, MacOS Tiger

Please provide any additional information below.

I suspect this was meant to be a feature and not a bug. But it seems like
(a), (b), and (c) should all be equivalent. Instead, a newline in the node
changes the filtering from "accept all" to "accept nothing". If nothing
else, seems like this should be documented. 

I'd understand if you chose to do nothing about this, since the fix is
really changing the validation so that unspecified values for attributes in
the policy files default to giving error, which is a total pain (if
philosophically correct, given that this is a whitelist, after all). 

This bug is really about how treating a tag in ways that one could easily
think of as equivalent instead yields two very different behaviors, and
that this isn't documented anywhere.

All that aside, great job on AntiSamy!

Original issue reported on code.google.com by thedownw...@gmail.com on 12 May 2008 at 7:51

GoogleCodeExporter commented 9 years ago
Whoops. Forgot that *id* was a "common attribute". When I do:

        <tag name="b" action="validate">
            <attribute name="guh"></attribute>
            <attribute name="fehonly">
                <literal-list>
                    <literal value="feh"/>
                </literal-list>
            </attribute>
            <attribute name="intonly">
                <regexp-list>
                    <regexp name="integer"/>
                </regexp-list>
            </attribute>
        </tag>

I get:

class org.owasp.validator.html.PolicyException
Attribute 'guh' was referenced as a common attribute in definition of 'b', but 
does
not exist in <common-attributes>

So I guess the real issue, like I wrote earlier, is the lack of documentation on
these nuances.

Original comment by thedownw...@gmail.com on 12 May 2008 at 8:07

GoogleCodeExporter commented 9 years ago
While that behavior is not ideal because the error message is misleading, I do 
not
see any needed action.

Thanks!

Original comment by arshan.d...@gmail.com on 25 May 2008 at 1:31

GoogleCodeExporter commented 9 years ago
Reference comment #2.

Original comment by arshan.d...@gmail.com on 25 May 2008 at 2:03

GoogleCodeExporter commented 9 years ago

Original comment by arshan.d...@gmail.com on 2 Jun 2008 at 7:30