!!! glsl.heroku.com has been hacked

[Kostadin]

Hello Mr. Doob,

I am sorry I have to use the long way of sending you a pull request to tell you this. glsl.heroku.com has been hacked for 2 days now. Open it and you will see. Currently, after a brief pause, it changes the thumbnails of the gallery to ponies that do "The Harlem Shake" (which as you know is not the REAL Harlem Shake but whatever). I am very grateful for the work you have put in the GLSL sandbox so telling you is the least thing I can do.

[mrdoob]

Yep. I'm aware it's been hacked. I find it disgraceful that even if the site is open source the guy (or girl) opted to show the problem that way instead of fixing the source here.

[emackey]

Agreed it's disgraceful, especially for an open-source site.

@jfontan can you remove the offending "images" from the DB? Also we should add server-side sanity checking to prevent this stuff in the future. Maybe a whitelist of characters allowed in the image data, or scan the data for single- and double-quotes.

[jfontan]

I'm back home already. I'll try to fix this mess soon.

The code is not secure and never intended to be so but after these couple of "jokes" I think I need to add some sanity check. I still think that adding users or more security is out of scope for the page.

[mrdoob]

I'm back home already. I'll try to fix this mess soon.

Many thanks!

I still think that adding users or more security is out of scope for the page.

Agreed.

[emackey]

Although the home page appears clean at the moment, pages 1 and 2 still have the hack. Specifically, these effect numbers need to be removed from the DB:

7853,
7852,
All effects from 7798 through 7823, inclusive.

Of these, 7823 has the worst script attached, and is the most important to remove. The script tag has a src link to DropBox and has been updated several times over the weekend by its author.

The scripts are simple HTML injection, just adding a close quote and close brace to the thumbnail image data, followed by arbitrary HTML. A bit of sanity checking should protect against this, without need for user accounts or high security.

@jfontan I imagine you're still catching up after getting home, but it would be great to have this fixed when you get a chance. Thanks!

[jfontan]

I've just deleted the offending items and added a little sanity check. I've tested three ways of preventing this:

Check that the image conforms to png data (I think this is the most correct one):

def img_regexp(img)
    img if img.match(%r{^data:image/png;base64,[\w+/=]*$})
end

Delete ", ', < and >:

def img_tr(img)
    img.tr(%q{"'><}, '')
end

And check for one of those characters:

def img_index(img)
    img if !img.index(/["'><]/)
end

Deleting the characters seems to be the fastest and is the one implemented now. Any comment is welcome.

               user     system      total        real
regexp:    1.200000   0.500000   1.700000 (  1.696879)
tr:        0.110000   0.000000   0.110000 (  0.123155)
index:     0.360000   0.000000   0.360000 (  0.357593)

[mrdoob]

FTW!

[Kostadin]

I do agree that deleting the characters is the fastest way to prevent the HTML injection. However, would it not be better to save everything in the database as is but sanitize all strings only when they are about to be displayed?

& should be replaced with &amp; < should be replaced with &lt; > should be replaced with &gt; If the string is put in a HTML attribute, the " symbol should be replaced with &quot; If the string is put in a single quoted Javascript string, the ' symbol should be replaced with &#39;

This way all strings will be displayed without visible loss of information and the HTML injection will not work.

[emackey]

I guess the new code isn't posted to GitHub yet? The sanity check should of course happen on the way into the database (during save or fork) so that it isn't needed on the way out (for every thumbnail ever displayed). Perhaps that's obvious but I wanted to double-check.

[emackey]

Nevermind my previous comment. Code is posted, and looks OK to my non-Ruby-programmer eyes. Unless there are strange Unicode quotation characters that can end the image data string, I think we're good.

@Kostadin we don't need full HTML encoding because there can't be any HTML in the image data.