Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default unicode escape in JsonResponse #32608

Closed
Guigoz opened this issue Apr 30, 2020 · 1 comment
Closed

Default unicode escape in JsonResponse #32608

Guigoz opened this issue Apr 30, 2020 · 1 comment

Comments

@Guigoz
Copy link

Guigoz commented Apr 30, 2020

  • Laravel Version: 7.8.1
  • PHP Version: 7.2.29
  • Database Driver & Version: N/A

Description:

Hi!

The Illuminate\Http\JsonResponse class extending the Symfony\Component\HttpFoundation\JsonResponse class, overwrites the default encodingOptions with 0 value.

    public function __construct($data = null, $status = 200, $headers = [], $options = 0)
    {
        $this->encodingOptions = $options;

        parent::__construct($data, $status, $headers);
    }

The Symfony defaults are more secure than the 0 value defined by Laravel.

class JsonResponse extends Response
{
    // Encode <, >, ', &, and " characters in the JSON, making it also safe to be embedded into HTML.
    // 15 === JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_AMP | JSON_HEX_QUOT
    const DEFAULT_ENCODING_OPTIONS = 15;

    protected $encodingOptions = self::DEFAULT_ENCODING_OPTIONS;

The "0" encodingOptions is not safe when the data is embeded into html

The same problem has been reported to Express : expressjs/express#3268

I know this problem is not easily exploitable with direct API call in web browsers and in all other cases, users should not print json in html context without html encoding it first.
But we never know how data will be used and frameworks should have defaults as secured as possible.

In this case, the default encoding options used by Symfony are more secure than the default 0 value of Illuminate\Http\JsonResponse.

Steps To Reproduce:

class TestController extends Controller
{
    public function test()
    {
        return Response::json('<script>alert("abc");</script>');
    }
}

It should return

"\u003Cscript\u003Ealert(\u0022abc\u0022);\u003C\/script\u003E"

Instead of

"<script>alert(\"abc\");<\/script>"

Thanks. :)

@taylorotwell
Copy link
Member

This is asking for a behavior change and is not a "bug" we can fix on an existing patch release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants