search

godotengine / godot-asset-library

PHP frontend for Godot Engine's asset library
https://godotengine.org/asset-library
MIT License
292 stars 89 forks source link

Check for XSS in templates #9

Closed Marqin closed 8 years ago

Marqin commented 8 years ago

I've looked a little at code and I don't see any usage of htmlspecialchars() nor regexps for escaping HTML/JS - does it mean, that every user can put their own JS (including trojan horse, etc.) into asset name/describtion/etc?

bojidar-bg commented 8 years ago

htmlspecialchars is indeed used in some of the templates, but a throughout check would be much welcome indeed.

bojidar-bg commented 8 years ago

Also, note that any JS included would not execute on the C++ frontend side, so the target is slightly limited to those that use the html frontend. Unfortunately this includes the moderators, so... :smile:

Marqin commented 8 years ago

According to template engine you use, they say they don't do any escape - https://github.com/slimphp/PHP-View/blob/master/README.md

bojidar-bg commented 8 years ago

Yeah, we do manual escaping. Maybe exposing a function e(string) to wrap echo htmlspecialchars(string) and ease the usage would be good...

Marqin commented 8 years ago

name e(string) may suggest it's something with error not escape

akien-mga commented 8 years ago

Yeah better use explicit function names than shady one-letter names :) Something like echo_escaped or secho (secure echo) might be better already.

bojidar-bg commented 8 years ago

Yeah, makes sense too. Would see what can be done later.

bojidar-bg commented 8 years ago

I escaped all templates, I just want to have a better check on some of them, after which I might push it.