search

tcowans / owasp-java-html-sanitizer

Automatically exported from code.google.com/p/owasp-java-html-sanitizer
Other
1 stars 0 forks source link

Stackoverflow sanitizing HTML with large inline background-image #16

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
Sanitize with ExampleTest.java the string below causes StackOverflow (with 
r173-EbayPolicy-based code), likely due to very deep regular expression tree. 
Ran into this sanitizing a large email collection. Verified with clean r176 by 
adding a test to ExamplesTest.java. If I sufficiently shorten the image data, I 
no longer get stack overflow.

1) testDataImage(org.owasp.html.ExamplesTest)java.lang.StackOverflowError
    at java.util.regex.Pattern$6.isSatisfiedBy(Pattern.java:4763)
    at java.util.regex.Pattern$CharProperty.match(Pattern.java:3345)
    at java.util.regex.Pattern$Branch.match(Pattern.java:4114)
    at java.util.regex.Pattern$GroupHead.match(Pattern.java:4168)
    at java.util.regex.Pattern$Loop.match(Pattern.java:4295)
    at java.util.regex.Pattern$GroupTail.match(Pattern.java:4227)
    at java.util.regex.Pattern$BranchConn.match(Pattern.java:4078)
    at java.util.regex.Pattern$CharProperty.match(Pattern.java:3345)
    at java.util.regex.Pattern$Branch.match(Pattern.java:4114)
    at java.util.regex.Pattern$GroupHead.match(Pattern.java:4168)
    at java.util.regex.Pattern$Loop.match(Pattern.java:4295)
    at java.util.regex.Pattern$GroupTail.match(Pattern.java:4227)
    at java.util.regex.Pattern$BranchConn.match(Pattern.java:4078)
    at java.util.regex.Pattern$CharProperty.match(Pattern.java:3345)
    at java.util.regex.Pattern$Branch.match(Pattern.java:4114)
    at java.util.regex.Pattern$GroupHead.match(Pattern.java:4168)
    at java.util.regex.Pattern$Loop.match(Pattern.java:4295)
    at java.util.regex.Pattern$GroupTail.match(Pattern.java:4227)
    at java.util.regex.Pattern$BranchConn.match(Pattern.java:4078)
    at java.util.regex.Pattern$CharProperty.match(Pattern.java:3345)
    at java.util.regex.Pattern$Branch.match(Pattern.java:4114)
    at java.util.regex.Pattern$GroupHead.match(Pattern.java:4168)
    at java.util.regex.Pattern$Loop.match(Pattern.java:4295)
    at java.util.regex.Pattern$GroupTail.match(Pattern.java:4227)
    at java.util.regex.Pattern$BranchConn.match(Pattern.java:4078)
    at java.util.regex.Pattern$CharProperty.match(Pattern.java:3345)
    at java.util.regex.Pattern$Branch.match(Pattern.java:4114)
    at java.util.regex.Pattern$GroupHead.match(Pattern.java:4168)
    at java.util.regex.Pattern$Loop.match(Pattern.java:4295)
[...repeats nothing more useful at bottom of list...]

The test is (String all on one line):

  public final void testDataImage() {
        String input="<a class=\"atc_s addthis_button_compact\" style=\"background-image:url(data:image/gif;base64,R0lGODlhMgCaAPf/AO5ZOuRpTfXSyvro5Pz08uCEb+ShkeummPOMdPOqmdZdQvbEud1UNuqmlvqbhchNMfnTyvXPxu6pmtFkTeJ6Y9JxXPR8YNRSNvyunP3Mwd1zW+iCau9dPtBOMe69sutwVPGGbuFcPvmRefqAZuhiQ/rJv9pFJf3h2up0WttCItmBbv3r5/26q/bc1v3XzvHOx8tML/nf2cpAI+hfQf3GufVbOvyrmvCkk/7r5vnm4t+BbPism/apmN9DI+hNLrkrD/x4V98+HeFBIPt0U/x2VflaOfdwT/lzUvhYN/p2VfhWNd47Gvx5WPtyUflcO+JEI/VsS/FjQvRpSPtwT/tuTebm5vpmRfppSPlePeRIJ8XFxdRQNPpsS8I9IYyMjOhQL+xYN+ZMK/liQflgP+1dO+pUM//188RBJZGRkff39/718vFZOcA/JtVXO8VEKPvVzeuhkPynk/ermsdHK/7Yz/ehjvi2p9Z4ZPre2NRDJPVkRPzz8d9NLdR1YNM2F/SkksBBKOdXOfaHbdhsVPFbO81cQs1UOO2HcPNTM/708dd7Zt1GJuFRMd1JKfBiQvyNcuF1XfqOd+6gj+SDbPuJb/re1/TUzeReP+mHce1zWeOdjthYPOaSgO2LdOyllPFzWPezpPe7rfVzVO6Hb/vz8f7f2eGJdOKMee54XeBiR/Z5XeNlSfyVfPJtT++ikf7i3POrnPGun9+VhOVSMvzo499/as1EJup5YPvUy+abi+tjRPNrS91aPfCCafyxn/SCaNVZPeu7r+B+ad1WOOuypOm3rPt6W/OikPvq5vCmlt6Qf/ygivp2V/OTfPrb1eKfkP7Z0OyBaeibiuieju+ciu2ejeKCbOatn+2YhOFVN+eRfedWNvWplvLRyuF4Xs5kTdlOL+3Eu+1mR/zp5fzq5dVMLtJTOPvDtvKgjvy1pe9yVchFKeafj+WYh/nBs/HIvv3Ctd9YONdVOM5BI8pXP/jUzOqKdOiMd9toUN1iSONuU8HBwYmJifX19f///////yH/C1hNUCBEYXRhWE1QPD94cGFja2V0IGJlZ2luPSLvu78iIGlkPSJXNU0wTXBDZWhpSHpyZVN6TlRjemtjOWQiPz4gPHg6eG1wbWV0YSB4bWxuczp4PSJhZG9iZTpuczptZXRhLyIgeDp4bXB0az0iQWRvYmUgWE1QIENvcmUgNS4wLWMwNjEgNjQuMTQwOTQ5LCAyMDEwLzEyLzA3LTEwOjU3OjAxICAgICAgICAiPiA8cmRmOlJERiB4bWxuczpyZGY9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkvMDIvMjItcmRmLXN5bnRheC1ucyMiPiA8cmRmOkRlc2NyaXB0aW9uIHJkZjphYm91dD0iIiB4bWxuczp4bXBNTT0iaHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wL21tLyIgeG1sbnM6c3RSZWY9Imh0dHA6Ly9ucy5hZG9iZS5jb20veGFwLzEuMC9zVHlwZS9SZXNvdXJjZVJlZiMiIHhtbG5zOnhtcD0iaHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wLyIgeG1wTU06T3JpZ2luYWxEb2N1bWVudElEPSJ4bXAuZGlkOkEyRDgxODE2MkMyMDY4MTE4NzFGRDNDMzU5QkE3OTE3IiB4bXBNTTpEb2N1bWVudElEPSJ4bXAuZGlkOkMzMjA1M0I4QkM4RjExRTBCRDBEQkE0MTlGMTc4MDZGIiB4bXBNTTpJbnN0YW5jZUlEPSJ4bXAuaWlkOjlFQzFFMTZFQkM4RDExRTBCRDBEQkE0MTlGMTc4MDZGIiB4bXA6Q3JlYXRvclRvb2w9IkFkb2JlIFBob3Rvc2hvcCBDUzUuMSBNYWNpbnRvc2giPiA8eG1wTU06RGVyaXZlZEZyb20gc3RSZWY6aW5zdGFuY2VJRD0ieG1wLmlpZDpDMjFGMUIwQjMyMjA2ODExODcxRkQzQzM1OUJBNzkxNyIgc3RSZWY6ZG9jdW1lbnRJRD0ieG1wLmRpZDpBMkQ4MTgxNjJDMjA2ODExODcxRkQzQzM1OUJBNzkxNyIvPiA8L3JkZjpEZXNjcmlwdGlvbj4gPC9yZGY6UkRGPiA8L3g6eG1wbWV0YT4gPD94cGFja2V0IGVuZD0iciI/PgH//v38+/r5+Pf29fTz8vHw7+7t7Ovq6ejn5uXk4+Lh4N/e3dzb2tnY19bV1NPS0dDPzs3My8rJyMfGxcTDwsHAv769vLu6ubi3trW0s7KxsK+urayrqqmop6alpKOioaCfnp2cm5qZmJeWlZSTkpGQj46NjIuKiYiHhoWEg4KBgH9+fXx7enl4d3Z1dHNycXBvbm1sa2ppaGdmZWRjYmFgX15dXFtaWVhXVlVUU1JRUE9OTUxLSklIR0ZFRENCQUA/Pj08Ozo5ODc2NTQzMjEwLy4tLCsqKSgnJiUkIyIhIB8eHRwbGhkYFxYVFBMSERAPDg0MCwoJCAcGBQQDAgEAACH5BAEAAP8ALAAAAAAyAJoAAAj/AP+h8EGwoMGDCBMqXOgDxb8ZUphInEixosWLGDMykTLDB5CPIEOKHEmypEmQBImoXMmypcuXMGOuJDikps2bOHPq3MnTJsEmQJv4G0rUX9CjSJMqXXqU4JSnRaM+nWKMxtBXy6Y8gvZoqtevYL8SpEK2KJWiCsjSMIPNlCUC6lj5u7eLrN27ePOSJcilb1EuaPvSQaZBnAUJGkT487DCTBwulOj4M8OCSxw6vvwxYzH5cV8uBK+IHiq69FBgoh0MPcHCAgnF7+zFIKArw4kAufxFw+AvBixiODo18IeqNEEryItaKdoGuRUEHgj4I6Aqkr8CIeT422QF0wEX13f4/xvEi1y9A9ob6EF+PDnR5USbW8EQydElaf4aWC/giHcbeGbYkcB12rWhhxkCSJBAAgXMwJ4PYkQYVVEXRIgDHp+IMYI/7Rzijw4c2ODPBf4kIw81H/IwohgZCJDKBxAcwkGEBI1h4xgTjniBjSMMQBQuF4DwYYgjpuMPBKX4c4qKO44wzlAlDMOBjQRhYaWVW2Sp5RZXYkEIJLUosAUDWACwBQBYBMLlLfgwwMCZalr5pQZbZHMlQU7kqeeefOq5BgA19LnnGoEK6sQaa+xJUBGMNuroo5BGKumkjRKExKWYZqrpppx26immPsyAiBKklmrqqaimquqqSiAyw0MMxf8q66yv/lPID7jmquuuvPbq668/FPIPGyGcZOyxyIbAxg9JNOvss9BGK+201DqL6xHYZqvtttx26+232eJqxLjklmvuueimqy65uELhLhQ5vivvvPTWa6+8uEqhb45D6SuFKOes5oAUgrggiL8IJ6xwwrhG4bBZRRnisDtqaKNCNwQEgMB1Mzjs8ccgh+xwww8TBRhREkfxxgCDXIJCA4NsHAw5atQRxS9v+KOGHVHU8YZ4rdihs80e40rG0aSVdsVQDxzdzFB4gPLBMKP4E441LewRTwmVKCCLP95w408LnlwzziQG+KPP0WTgCsbbyhUFw9tgbFAMKf7s8YGQimz/cYM/cwtjAAT+KPL3BB0MIIABfz+zzdu4liF53ETNIfkfvTBSjjL+aBKNP3cw8oc/c4SSSCxwgO4K6bMk8gI7cMBxBziS4/rF7fz648bttLSwyheZ+KMMBf70wccxuvszzTqcFC+J7l8s8II5+URAAR+34xrG9mHkeMYZ23+QA1ERnEF8H42g488ZsPgTgTP+qFDN+mGIP9QCtjSyPa5Z9N9/FwAMYBf8l4VFTKACD+iCDLJggi6YIAt5GCA+6CEDGTgwgv0z4De6MA//4eoJIAyhCEcYwhSkoAckFOEJUwhCE4oQV0KIoQxnSMMa2vCGOJQhroLAwx768IdADKIQ/4fYwx8Awg9LSKISl8jEJjrxiVBcgh8AMSxgWfGKWGTDP/6hhX148YtgDKMYx0jGMu5DC1zsR+7WyMY2FqUfXXSjHOc4IS/S8Y5ztCMe97hGPfLxj1HxIyAHKchB/rGQhtwjIhN5x0UyMo/7eOQhIylJRVKyko28JCYhuclMdpKOjvxkjkIpykBqspS5IyUqh6LKVbYSla8sZSxFOctP1rKTt9xkLjG5y0r2UpJnTMMqc5cGNHbRjGVU4xuRSUY0bvGZ0IymNLe4D2X6ox/7mKY2t8lNblbzmtnspjjHuc1IhpOc6EznP86pzmiigR/wjKc850nPetrznvxAwz+8UP+FfvjznwANqEAHStCC9qMKXuCHQRfK0Ib6E54OjahE/wnRiVqUoRW9qEYHmtGNevShCv2oSDsqUo2StKQWPSlKJarSlTq0pS7FaEhjOlGY0rSgNr0pR2eq04bmtKcA/SlQQTpUn/K0qAQV6lCVClSm9tSpOoXqTaVKU6rG1KouxepKtYpSfvATqQVF6D7xic9+ArQKZLWnF9oZTX6Y9aD8YKtc29pPtM71rs90a1zxyte98vWZx2SmYL/oTHRqwZqPhGM6T/lIdoqTsYx0bDchm0jJehOXi8UsOb+py80i1pfj5KxmvflZXoa2tMD0rC3RKVrQsna0oYXtY2U7WdplclMLwqxkMdUZ2MEKtrDofGdahytPfabzqzEVKzqPutV0Mrerzo1qdKc63aou961XJadbnzrO7XK3m979Lnixa93ukjer15XuctWrXfZ2173ifG5J/TpO5LpUuehMKHH3u1ZxBgQAOw==);\"></a>";
    String sanitized = EbayPolicyExample.POLICY_DEFINITION.sanitize(input);
    System.out.println(sanitized);
  }

What is the expected output? What do you see instead?
Sanitized HTML with this style monstrosity removed or passed.

What version of the product are you using? On what operating system?
r173-based code, repeated with test case in clean r176.

$ java -version
java version "1.6.0_45"
Java(TM) SE Runtime Environment (build 1.6.0_45-b06)
Java HotSpot(TM) Server VM (build 20.45-b01, mixed mode)

Linux 3.2.0-49-generic-pae #75-Ubuntu SMP

Attached gz version of smallest HTML fragment that causes error (image data can 
be shortened further, but attached is with original image data).

Thanks!

Fred

PS:

FWIW -
in StylingPolicy.html, the '.' in the below regexp should probably be escaped. 
However, doing so does not help with above problem (also doesn't cause new 
failures):

  private static final Pattern NON_NEGATIVE_LENGTH = Pattern.compile(
      "(?:0|[1-9][0-9]*)([.][0-9]+)?(ex|[ecm]m|v[hw]|p[xct]|in|%)?");

I also though in CssGrammar.java, the URL_CHARS regexp should require at least 
one character (didn't check with actual CssGrammar, though) by makeing '*' into 
'+', but that also did not abolish the problem (also doesn't cause new 
failures):

    // url chars               ({url_special_chars}|{nonascii}|{escape})+
    String URL_CHARS = "(?:"
        + url_special_chars + "|" + nonascii + "|" + escape + ")+";

Original issue reported on code.google.com by fred.lin...@gmail.com on 16 Jul 2013 at 2:51

Attachments:

GoogleCodeExporter commented 9 years ago 
Is the image copyrighted?

Thanks for the note on the regexs.  I have a rewrite of the CSS lexer mostly 
ready to go which gets rid of regular expressions (and the associated unbounded 
recursion and backtracking) entirely.

Original comment by mikesamuel@gmail.com on 16 Jul 2013 at 4:22

GoogleCodeExporter commented 9 years ago 
Great!

Copyright - I don't know.

It looks like someone sent this page:
http://www.thesouthafrican.com/entertainment/south-african-couple-launches-engli
sh-bubbly-to-rival-champagne.htm

and that the image is the red "+share" icon at the top. Attaching as both 
base64 (from the html) and decoded to gif.

Original comment by fred.lin...@gmail.com on 16 Jul 2013 at 9:56

Attachments:

GoogleCodeExporter commented 9 years ago 
Attached, Untitled.zip that includes Untitled.html, Untitled.b64, and 
Untitled.gif that I made using gimp. It triggers the same error.

I hereby release this to the public domain without any warranty explicit or 
implied.

Original comment by fred.lin...@gmail.com on 16 Jul 2013 at 10:01

Attachments:

GoogleCodeExporter commented 9 years ago 
Great.  Thanks.

Original comment by mikesamuel@gmail.com on 16 Jul 2013 at 10:38

GoogleCodeExporter commented 9 years ago 
https://code.google.com/p/owasp-java-html-sanitizer/source/detail?r=180 
replaces the CSS lexer with one that passes your test, and doesn't backtrack 
but I'm not going to close this bug until that's production ready since the new 
code has not been thoroughly vetted on malformed inputs.  

My plan thus far is

1. Rewrite the CSS filter with a token-level filter based on Caja white-lists ( 
https://code.google.com/p/google-caja/wiki/CajaWhitelists ) that conservatively 
identifies all URLs, and normalizes tokens.  This should fix all the border 
problems by being more permissive and put us in a place to allow data URLs.
2. Test the new lexer with fuzzers and white-box tests until I'm confident that 
there's no inf. loops.
3. Push a release with the more liberal CSS sanitizer.
4. Look into data: URLs and plan from there.

I should be able to get 1-3 done this week or next, but feel free to play 
around with trunk in the meantime but please don't roll out to production yet.

Original comment by mikesamuel@gmail.com on 17 Jul 2013 at 12:24

GoogleCodeExporter commented 9 years ago 
Re data: attributes, I'm unsure what to do there.

https://www.owasp.org/images/0/03/Mario_Heiderich_OWASP_Sweden_The_image_that_ca
lled_me.pdf suggests that allowing images is not ok, and I don't know whether 
browsers agree on the origin of an image from a data URL.

I could whitelist

    data:image/gif;base64,...
    data:image/png;base64,...
    data:image/jpeg;base64,...

where the first 4 characters of ... are the b64 encoding of the first 3 
characters of the magic number for that image type.

That doesn't eliminate the risk of polyglots like 
http://www.thinkfu.com/blog/gifjavascript-polyglots but combined with the 
explicit mime-type should suffice.

Original comment by mikesamuel@gmail.com on 17 Jul 2013 at 10:20

GoogleCodeExporter commented 9 years ago 
Tested with trunk/r198 and can confirm that the stack overflow no longer 
happens. For my application, I don't miss the image. We want to show email 
reasonably faithfully, but removing attack vectors in much more important that 
look and this type of inline image data is rare (it was one document in about 
150,000).

Original comment by fred.lin...@gmail.com on 18 Jul 2013 at 7:50

GoogleCodeExporter commented 9 years ago 
Stackoverflow is fixed at r198.

Punting on support for data URLs until there is a client who really needs image 
embedding via CSS.

Original comment by mikesamuel@gmail.com on 24 Jul 2013 at 3:47

GoogleCodeExporter commented 9 years ago 
Thanks, Mike!! OK in testing. Will use in prod as soon as it shows up on maven 
central.

Original comment by fred.lin...@gmail.com on 24 Jul 2013 at 4:20