Fix for #1046 (Custom JS is being escaped)

[Gerrit0]

Fix for #1046 (Custom JS is being escaped)

The cause of this bug was bc202b5, as everything was encoded when first fetched, nearly every input was encoded twice, and the JS was encoded. The fix was to remove this, and also check everywhere Input::get() was used to ensure that encoding wasn't missed (nowhere...).

Changes proposed

• Revert bc202b5, which encoded all input
• Remove decoding of input when updating post as it is no longer needed.

[CraigChilds94]

@Gerrit0 I'm trying to see if this will impose any issues.

[TheBrenny]

I guess this is being held open because these were put in place to stop a stored XSS attack... Because should someone gain access to the admin panel, they can write a script in posts, <script>alert(123);</script>, to conduct malicious activities. I'll clone this PR and run it, and we'll see what happens - fingers crossed. 🙏

[Gerrit0]

That is intended behavior. HTML should be allowed in posts, and there is even a field for custom JS. The XXS problem is with comments, which should be properly escaped.

[TheBrenny]

@Gerrit0, see this. This is why literally everything is encoded...

[Gerrit0]

I totally get that, really, I do. It's a PAIN to deal with XSS, but the thing is, it shouldn't be handled by the class we use to get the input.

Remember magic_quotes_gpc? It seemed like a great idea at first - auto escape all that input from the user so it doesn't bite us when inserting it into the database. I would argue that this is the exact same mistake. Just substitute database for page and quotes for HTML chars. get_magic_htmlchars_gpc.

Didn't mean to turn that into a rant - I believe it makes my point well though.

Edit: This has some good examples.

Also, take a look at the 1.0 branch - Mustache auto escapes output, as everything is currently stored encoded, at the very least a migration script will have to loop through every stored item in the database and unescape it so that we don't end up with double escaping.