guillaumebriday / laravel-blog

Laravel 11.0 blog application with Hotwire, Horizon, Telescope, Sanctum, Breeze and Pusher
https://laravel-blog.guillaumebriday.fr
MIT License
1.75k stars 572 forks source link

Added regex to Auth/RegisterController because of Stored XSS vulnerability. #8

Closed x1mdev closed 6 years ago

x1mdev commented 6 years ago

I have added regex:[A-Za-z1-9 ] to the Validator function inside the RegisterController.php controller.

Without this regex in the validator a client could register with a username that allowed special characters, resulting in a stored XSS. The user name is stored in database from which even more damage could be done should there be malicious code being called by the XSS.

guillaumebriday commented 6 years ago

Hey ! Thank you for your work ! 👍

However, I think the XSS attack can be prevent by escaping data with Blade. The documentation says https://laravel.com/docs/5.6/blade#displaying-data

I close this PR because I don't want to limit user's name to this characters only.

x1mdev commented 6 years ago

Oke, how would you suggest implementing the prevention of escaped data with Blade? Tried some of the techniques from the link you send but doesn't seem to work.

Another finding: When placing a comment with this content: {{ alert(2) }} an XSS alert box will pop when viewing the public profile of the user that placed the comment.

This also pops in the admin dashboard when checking the comments list.

Any ideas?

guillaumebriday commented 6 years ago

If you have a name or a comment with the content <script>alert('3')</script>, It's just gonna print <script>alert('3')</script> without execution of the code the same way you see the text on GitHub and not an alert with 3.

It's because I use {{ $user->name }} instead of {!! $user->name !!}

We have to save the user's text in the database but we just need to escape data in the view.

For exemple :

screen shot 2018-02-15 at 15 22 15

Everything works fine here 👍

x1mdev commented 6 years ago

You are correct on the <script>alert('3')</script> part. But when you use the payload {{ alert(1) }} in the username, post title or comment field the alert box will pop. It also pops on the admin dashboard, making this a target for blind XSS. I have tested this and got a screenshot of the logged in admin dashboard using XSShunter payload.

I have edited the {{ $user-name }} to {!! $user-name !!} and it still pops an alert message. You can check it on https://x1m.nl/blog , there is a live Proof of Concept there.

https://x1m.nl/blog/users/4 also has the xss vuln when escaped with {!! !!}

snapey commented 6 years ago

add v-pre to any element that can contain user supplied content to prevent {{...}} being parsed as javascript by Vue

guillaumebriday commented 6 years ago

@x1mdev 🤔 Ok I got it. Could you open an issue, I'll fix it as soon as possible !

Thank you 👍

guillaumebriday commented 6 years ago

@snapey Hum, I think it could fix the problem but I don't want to use Vue on each element :/

x1mdev commented 6 years ago

I'll open an issue!

snapey commented 6 years ago

You only have to add v-pre on elements that are echoing user supplied input for instance, a template might have

<comment>
    <div v-pre>{{ $comment->body }}</div>
</comment>