Flash messages vulnerable to XSS attacks

Summary

Adding messages to the Phalcon\Flash\Session object will cause the contents of the message to remain unecaped when they are displayed, possibly allowing an attacker to inject HTML into the page (XSS). Tested with phalcon

  • Class affected: \Phalcon\Flash\Session
  • Method used: {{ flashSession.output() }}

System:

  • Phalcon version: 2.0.10
  • OS: Tested on Ubuntu

Code examples:

// Setting the session in my dependency injection object
$di->set('flashSession, function() {
    return new Session([
        'error'   => 'alert alert-danger',
        'success' => 'alert alert-success',
        'notice'  => 'alert alert-info',
        'warning' => 'alert alert-warning'
    ]);
});

// Adding messages inside of a controller
$this->flashSession->success("<script>alert('This will execute as JavaScript!')</script>");

// Echoing out the messages in a volt template (the message is printed as HTML)
{{ flashSession.output() }} 

// Same result
{{ flashSession.output() | escape }}

// Also the same result
{% autoescape true %}
    {{ flashSession.output() }}
{% endautoescape %}

IMHO flash messages is generated by your software in server side, not by user from client side. Sanitizing user's input first before take any actions, e.g: striptags, htmlentities, filter, etc.

Anyway, you can sanitize like this if you want

$this->flashSession->success(htmlentities("<script>alert('This will execute as JavaScript!')</script>"));
edited Feb '16

If a message contains part of the user input, eg: $this->flash->error("Value $msg is not valid for field something"); and the developer writing the application has no prior knowledge that this method might cause an XSS vulnerability, it seems like a better idea to sanitize it by default and maybe allow a toggle to explicitly allow HTML input, than allow it by default.

IMHO flash messages is generated by your software in server side, not by user from client side. Sanitizing user's input first before take any actions, e.g: striptags, htmlentities, filter, etc.

Anyway, you can sanitize like this if you want

$this->flashSession->success(htmlentities("<script>alert('This will execute as JavaScript!')</script>"));

Another question will be: Why PHP itself didn't sanitize every user input by default?

The answer is very clear: When it comes to input sanitization, one size does not fit all. So I think that's the reason Phalcon too.