4
\$\begingroup\$

The goal of this question:
Your goal here is to find a security hole in my code which allows a user to create input that contains a script doing anything they want, without that script being stopped by my code.

Please bear in mind that that is the ONLY goal of this post. I am not so much here to talk about how good the algorithm is, only if it works. However, that being said, I am still open to ideas on how to improve the algorithm, make it faster, etc, but please as comments. I will upvote any good suggestions :)

With that out of the way,
My code:
As you may have read above, you are trying to trick my code into letting a script through. What does that mean exactly? Well, I have created a php script which attempts to parse HTML and remove scripts and embeds which are not from trusted websites. Yes.... Using some regex... I know, but I do have a good reason -- You see, the only reasons I have found and can think of NOT to use regex are:

HTML is recursive, regex is not. (hence all this talk about context-free vs regular. I am sure there are more reasons for HTML being "above regex" but the recursion is the best one I can persoanly come up with.)

However, the interesting thing about my problem is this: Scripts are not recursive! This means that WHENEVER I come across a script tag, everything between that and the next end tag will NEVER be html! Thinking of HTML as a bunch of random letters, with every once in a while an open-script tag and a close-script tag actually brings HTML down to a level that regex can handle. And that's almost exactly what I did...

The other reason is memory constriction. I've used a lot of non-capturing groups with my regex, so, that should not be a problem. I also don't use 100% regex, I only use it as a generic way to detect tags. Actual handling of the tags is done with my own code (only with a bit more regex to select src attributes)

The reason I decided to push so hard for regex is this:

  • DomDocument only handles CLEAN html. I will be receiving html form potentially malicious users. Clean html will very likely NOT be provided.
  • PHP HTML tidying libraries are not an option as they require installation to the backend, which I have no control over whatsoever.
  • Writing my own 100% substring-indexof interpreter would become a mess. Especially when having to deal with both embed and script tags. Regex expresses all of the same logic but much more concisely, which is why I chose to go with it instead.

So, there you have my reasons. If you attack my code for the regex, I would rather you not attack it without at least reading that section of the question above.

The question: With all of that said, I will clearly define what I need: I need to know if the following code has security holes. IE is there a way that a user could cause a script within the inputted text to be ignored and passed through? For testing, I consider a script that alerts the text "haxored" to be acceptable. You will find some example input at the bottom.

<?php
header('Content-type: text/plain');
function dbStr($string)
{
    $ranges = dbStr_GetRanges($string);
    return dbStr_FilterStringWithRanges($string, $ranges);
}

function dbStr_FilterStringWithRanges($string, $ranges)
{
    $offset = 0;
    $maxidx = 0;
    foreach ($ranges as $range)
    {
        if ($range[0] + $range[1] <= $maxidx) continue;
        if ($range[0] < $maxidx)
        {
            $orig = $range[0];
            $range[0] = $maxidx;
            $range[1] -= $range[0]-$orig;
        }
        $string = substr_replace($string, '', $range[0]-$offset, $range[1]);
        if ($range[0]+$range[1] > $maxidx) $maxidx = $range[0] + $range[1];
        $offset += $range[1];
    }
    return $string;
}

function dbStr_GetRanges($string)
{
    preg_match_all
    (
        "#<(/){0,1}?\s*?(?:script|embed)"."[^'\"/]*?(?:[^'\"/]*?[\"'](?:(?:\\\\\"|\\\\'|[^\"'])*?)['\"][^'\"/]*?)*?[^'\"/]*?"."(/){0,1}?>#imsSX",
        $string,
        $matches,
        PREG_SET_ORDER|PREG_OFFSET_CAPTURE
    );

    $ranges = array();
    foreach ($matches as $key=>$value)
    {
        if (!in_array($value, $matches))continue;
        $type = get_dbStrMatchType($value);
        $possiblesave = null;
        if ($type == 1)
        {
            $idx = strlen($string-1);
            $len = 0;
            $protectkey;
            foreach ($matches as $key2=>$value2)
            {
                if ($key2 < $key) continue;
                $type2 = get_dbStrMatchType($value2);
                if ($type2 == 2)
                {
                    $idx = $value2[0][1];
                    $len = strlen($value2[0][0]);
                    $protectkey = $key2;
                    break;
                }
            }
            $substrstart = $value[0][2] + strlen($value[0][0]);
            $content = substr($string, $substrstart, $idx - $substrstart);
            if (preg_match("#[^\s]#imsSX", $content))
            {
                $ranges[] = array($value[0][3], ($idx+$len)-$value[0][4]);
            }
            else
            {
                if (isset($protectkey))
                {
                    $possiblesave = $protectkey; 
                }
                $type = 3;
            }
        }

        if ($type == 2)
        {
            $ranges[] = array($value[0][5], strlen($value[0][0]));
        }
        else if ($type == 3)
        {   
            preg_match_all
            (
                "#src=[\"']((\\\\\"|\\\\'|[^\"'])*?)['\"]#imsSX",
                $value[0][0],
                $submatches,
                PREG_SET_ORDER|PREG_OFFSET_CAPTURE
            );
            if (count($submatches) !=1 || !approve_dbStrSrc($submatches[0][6][0]))
            {
                $ranges[] = array($value[0][7], strlen($value[0][0]));
            }
            else
            {
                if ($possiblesave != null)
                {
                    unset($matches[$possiblesave]);
                }
            }
            $possiblesave = null;
        }   
    }
    return $ranges;
}
function get_dbStrMatchType($val)
{
    if (count($val) == 3 && strcmp($val[2][0], "/")==0)
    {
        return 3;
    }
    else if (count($val) == 2 && strcmp($val[1][0], "/")==0)
    {
        return 2;
    }
    else
    {
        return 1;
    }
}
function approve_dbStrSrc($src)
{
    $dbStrTrusted = array
    (
        "http://www.youtube.com",
        "http://youtube.com",
        "http://widgets.twimg.com/",
        "http://www.twiigs.com/",
        "http://twiigs.com/",
        "http://twitter.com/",
        "http://www.twitter.com/",
        "http://picasaweb.google.com",
        "http://www.flickr.com",
        "http://flickr.com",
        "http://static.pbsrc.com/",
    );
    foreach ($dbStrTrusted as $trusted)
    {
        if (strpos($src, $trusted) === 0)
        {
            return true;
        }
    }
    return false;
}
echo "test" . dbStr
(
'
This will be passed:
<embed type="application/x-shockwave-flash" src="http://picasaweb.google.com/s/c/bin/slideshow.swf" width="288" height="192" flashvars="host=picasaweb.google.com&amp;hl=en_US&amp;feat=flashalbum&amp;RGB=0x000000&amp;feed=http%3A%2F%2Fpicasaweb.google.com%2Fdata%2Ffeed%2Fapi%2Fuser%2F109941697484668010012%2Falbumid%2F5561383933745906193%3Falt%3Drss%26kind%3Dphoto%26authkey%3DGv1sRgCN2H88H41qeT6AE%26hl%3Den_US" pluginspage="http://www.macromedia.com/go/getflashplayer"></embed>


This will not:
<script type="text/javascript">
    alert("U R HAXORED");
</script>
');
?>

If the full version interests you (E.G. the version filled with notes about how the algorithm works) then check this out:

http://pastebin.com/jYZH07cK

EDIT: Here is an online demo of the code: http://www.geiodo.com/g-cont/mars3/MarsSecurity.php

\$\endgroup\$
  • 2
    \$\begingroup\$ I'm pretty sure this is the wrong site for this. You might try your luck at Code Golf or SO or something. This site is only for reviews. If you want the code reviewed, that's fine, but saying that you ONLY want someone to break your code is against the FAQ \$\endgroup\$ – mseancole Sep 5 '12 at 18:26
  • 1
    \$\begingroup\$ Hmm, I apologize. I asked first on Meta, and people there seemed to feel that this was the right place. I will check the FAQ. meta.stackexchange.com/questions/145949/… \$\endgroup\$ – Georges Oates Larsen Sep 5 '12 at 18:27
  • \$\begingroup\$ I read the main part of the FAQ. Are you positive I am in violation? I would like to have peers review my code for a "Security issue". I would just prefer that over an algorithm change aimed at efficiency \$\endgroup\$ – Georges Oates Larsen Sep 5 '12 at 18:38
  • 2
    \$\begingroup\$ Now that you have reworded it, it looks a lot more in line to the FAQ, but the original wording made it seem that you were just challenging people to hack your code, which seemed more like a post for code golf. And now that I'm thinking more deeply about it, maybe that wasn't against the FAQ either, it just seemed poorly worded for what you truly wanted. Carry on \$\endgroup\$ – mseancole Sep 5 '12 at 19:03
  • 1
    \$\begingroup\$ @meanscole Thankyou for pointing out my wording! I wouldn't want to turn potential reviews away due to a simple mistake in writing on my part \$\endgroup\$ – Georges Oates Larsen Sep 5 '12 at 19:11
3
\$\begingroup\$

The filter doesn't block inline javascript.

Example 1:

<body onscroll=alert(1)><br><br><br><br><br><br>...<br><br><br><br><input autofocus>

Example 2:

<form id="test"></form><button form="test" formaction="javascript:alert(1)">X</button>

Also, it doesn't encode the html, thus this will break your filter. </textarea>

Example:

Inserts HTML:

</textarea><marquee><h1>I'm a bug</h1></marquee><textarea>

Inserts Script:

</textarea><script>alert(\'I'm a bug\')</script><textarea>

If you're trying to prevent XSS attacks then your goal should be not to allow ANY html to render. One way to do this would be to replace the < and > symbols with the respective special html entries, being &lt; and &gl;.

\$\endgroup\$
  • \$\begingroup\$ That was just a demo -- The idea is not to get code outside of the textarea, the idea is to get a script to appear INSIDE the text area. For instance, if you write <script>alert("hax05");</script> in the top text box, you will find that this does not appear in the bottom text box. This answer only breaks the demo I whipped up in 5 minutes -- NOT the underlying filter. Still, thankyou for the effort \$\endgroup\$ – Georges Oates Larsen Sep 6 '12 at 3:50
  • \$\begingroup\$ Aha! Just saw your update... Of COURSE... I missed inline js.... Thankyou! I will accept this as answer. I am writing this for a system which originally did not block anything at all. We went for years allowing ANY html whatsoever on posts... REmoving html 100% is something the creator is not willing to do, but i would agree, removing all HTML is definitely the way to go imo. \$\endgroup\$ – Georges Oates Larsen Sep 6 '12 at 8:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.