therecluse26 / PHP-Login

A login system built with PHP, MySQL, jQuery and Bootstrap
MIT License
839 stars 444 forks source link

XSS attack vulnerability #213

Open jeltevandervalk opened 6 years ago

jeltevandervalk commented 6 years ago

There seems to be no prevention of XSS attacks in this system.

Example: if a new user signs up with this username <script>alert('XSS');</script> the alert will pop up as soon as you open the validate/delete new users table. Someone could load external scripts on your page, and then who knows what could be done from there.

Could you implement XSS prevention, such as input and output filtering and or sanitisation?

uomopalese commented 6 years ago

Hi, you can use HTML Purifier. I'm using the "Lite" version. You can modify the top of login>ajax>createuser.phpas follow: <?php require '../../vendor/autoload.php'; require_once '../path/to/HTMLPurifier.auto.php'; try { //Pull username, generate new ID and hash password $newid = uniqid(rand(), false); $purifier = new HTMLPurifier(); $dirty_newuser = str_replace(' ', '', $_POST['newuser']); $newuser = $purifier->purify($dirty_newuser); ......

Hope this helps.

jeltevandervalk commented 6 years ago

@uomopalese Nice! This will perfectly do the job for the new user form.

I think however that for better security, methods like this one need to be implemented throughout the project. I saw that there are already some input filters in place, but I could be done better. Output filters may also be used where necessary, for example in the DataTables js initialisation files, see also this link about possibilities to extend (reusable) rendering functions to implement sanitisation.

The parameter binding and csrf tokens, among other features, show that this project, as far as I can check this, does a good job being secure. And for such a nice project which can serve as the starting point or example for many login systems, security is quite important. Especially since this "simple" login system can be used by relatively unexperienced developers who might assume this project is infinitely secure.

uomopalese commented 6 years ago

Yes, I see your point of view. I thought at first that you were simply looking for a shorthand to achieve your goal, but I agree with you, I'm using this login on a little project and I would like to see it more secure, for future jobs.