Fixes XSS vulnerability in Profiler #1305
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
taylorotwell
added a commit
that referenced
this pull request
Oct 3, 2012
Fixes XSS vulnerability in Profiler
|
Thank you! |
| @@ -148,6 +148,7 @@ public static function query($sql, $bindings, $time) | |||
| $binding = Database::connection()->pdo->quote($binding); | |||
|
|
|||
| $sql = preg_replace('/\?/', $binding, $sql, 1); | |||
| $sql = htmlspecialchars($sql); | |||
There was a problem hiding this comment.
What about HTML::encode() or the shortcut e() instead? htmlspecialchars() without the appropriate parameters will just cause other problems... e.g. with single quotes.
There was a problem hiding this comment.
I was thinking about using HTML::entities() would be fine. What's the difference?
There was a problem hiding this comment.
HTML::entities(). That's what I meant. Brilliant. :)
Just look at the implementation of the function, the parameters are quite important.
|
@taylorotwell |
zoe-edwards
pushed a commit
to zoe-edwards/laravel
that referenced
this pull request
Oct 14, 2013
Replace foreach with in_array
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The profiler doesn't sanitize the queries on the SQL tab which can allow an attacker to forge specially crafted queries to attack other visitors, steal cookies, and perform unauthorized browser actions. It does depend on how the developer has developed the application but nevertheless it should be patched. Even if the attack fails it can still render as HTML so the full query doesn't always show.
At first it was hard for me to provide a working PoC (Proof of concept) as the system adds slashes to the quotes in order to, I assume, prevent SQL injection (I haven't check if that's the database or the profiler's doing) but I did manage to forge this:
This results in:
http://i.imgur.com/VA4r2.png
But should be:
http://i.imgur.com/hebkV.png
This pull request fixes an XSS vulnerability in the Profiler. It shouldn't affect any other functionality as far as I'm aware.