Log in

View Full Version : Resolved security question


james438
06-06-2011, 02:15 AM
For user submitted css if I were to str_replace "style" would that effectively eliminate possible javascript injections?
djr33
06-06-2011, 02:29 AM
I don't understand. How could CSS involve Javascript? Why not just let them submit text that goes between the <style....> and </style> tags? I don't believe Javascript can operate in there. Maybe some browsers would allow it, so you might still need to check, but it's a good start.

As for replacing "style", do you mean replacing "<script>" instead? <style> is css, and <script> is JS.
That might stop a lot of it, but remember that Javascript can also be embedded in attributes like onclick, onmouseover, etc.

The safest way to eliminate any of that is to replace all instances of <, >, " and &. But that might also cause some problems with your CSS.

Maybe you meant that you don't want users to be able to end the <style> block of CSS by adding </style>. That's a good point. After that they could put JS. But I'd recommend not blocking "style", but instead "</style>" (AND possible variations on that) because "style" itself might be part of a class for example, and it is part of several CSS properties, like list-style.


Please post the exact format that you are allowing users to upload.
james438
06-06-2011, 03:14 AM
I think in the last major paragraph you were understanding my situation.

This is really just in the concept stage, but here is some more detail.

The user submitted css would be between the style tags. str_replace('script','',$css); would eliminate the closing script tag. I used "script" as opposed to "/script" because what if the user used "</ script>" or "< / script>" or </Script>, etc? I would have to make the str_replace case insensitive too I guess.

Removing <, >, &, " sounds like a good, if not better idea. I am pretty sure that it is easier to work around those 4 symbols easily enough. I am not a big fan of users being able to insert javascript into the css using the method mentioned above.
djr33
06-06-2011, 03:24 AM
I understand now. I think it might be best to approach this with regular expressions trying to block all variations of </style> tags and variations. (I think in your post above, maybe after reading my complicated response, you switched "style" and "script", right?).

You can probably eliminate < and > completely, but " can be used in CSS such as for paths. I'm not sure if & is ever used in CSS.
james438
06-06-2011, 03:31 AM
hehe, you're right, I did switch style and script around! silly me ;)

It looks like I need to use regular expressions here. I wanted to see if there was an easy way to do this without pcre though. My philosophy is to only use pcre if you really need to.

I think I will use both methods: remove > and < and use pcre to remove variations of "</style>".

Could using paths insert javascript?
djr33
06-06-2011, 03:42 AM
I don't think there's any way to use a path to insert Javascript. That would just apply, for example, an image as a background. I really doubt any browser would allow a .js file to be loaded there somehow.*

If you remove < and > then removing </script> is redundant. I'd suggest changing them to &gt; and &lt;. That way you'll end up with at worst &gt;/style&lt; and that's harmless.


*In theory, there are "javascript:" paths, but I don't know if they're allowed in CSS. I doubt it. We'll have to wait to see if someone else mentions something about that. You can always try the following:

<style type="text/css">
body { background-image: url("Javascript:alert();"); }
</style>
....I'm not even sure if that's the right syntax (if it actually might exist).
james438
06-06-2011, 03:48 AM
Thanks for the tips. I should be able to avoid using pcre that way too.
traq
06-06-2011, 03:51 AM
> is an operator in css (it means "children", as opposed to a simple space, which means "descendants"):

<style>
body div{ background: red; }
body > div{ background: blue; }
</style>
<body>
<div>
This div will have a blue background.
<div>
This div will have a red background.
<div>
So will this one. Without the > operator in the second style rule, all the divs would have been blue.
</div>
</div>
</div>
</body>granted, it's severely underused, so you might be perfectly "okay" with disallowing it (no one is likely to notice anyway :) ).
james438
06-06-2011, 03:56 AM
maybe if I just transformed "<" then?
traq
06-06-2011, 04:17 AM
why don't you just use strip_tags()? that was my first thought.
djr33
06-06-2011, 04:18 AM
Thanks for pointing that out, traq.
James, assuming there's no other use for < and > in CSS (I can't think of one, but that doesn't mean there isn't something), then I don't imagine that < would have any significance since that would be "parent" or something... probably irrelevant.

Just removing < would stop code injection. But it would allow for messy source as a result-- if they used <tag> for example, you'd have tag>, and that might confuse a parser. So it might not validate for example. But it would be secure... I think.


strip_tags() would probably be fine. Is there no case where tags could be used in CSS legitimately? I can't think of one.
traq
06-06-2011, 04:25 AM
I can't think of any case where html tags are used in css.

about the " < " parent operator: that would be awesome, really; but it does not exist. :(
james438
06-06-2011, 04:28 AM
I might be coding wrong here, but shouldn't the following only produce the word "test"?


<?php
$test="body { background-image: url(\"Javascript:alert();\"); }</style><script type=\"text/javascript\">
window.location = \"http://www.animeviews.com/\"
</script>";
$test=strip_tags($test);
echo "<html><head><style=\"text/css\">$test</style></head><body>test</body></html>";
?>

I am pretty sure I coded wrong here. Either way I need to sleep for a bit now.
djr33
06-06-2011, 04:35 AM
I think strip_tags() removes just the tags-- it'll leave anything before, after or between them.
traq
06-06-2011, 05:07 AM
yes, strip_tags() only removes the tags themselves (including their attributes, but not their contents). this means, however, that the contents of the script is simply rendered as text: safe enough.
james438
06-06-2011, 09:15 AM
I am using strip_tags and it works great.