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

[5.4] Escape inline sections #17453

Merged
merged 1 commit into from
Jan 20, 2017

Conversation

sileence
Copy link
Contributor

@sileence sileence commented Jan 20, 2017

Subtle but important change (imho).

This is a possible XSS attack: @section('title', '</title><script>alert("hi")</script>')

Of course the problem there is evident, but it is less evident if you have something like:

@section('title', $user->username) or @section('title', $userPost->title)

Using:

@section('title')
    {!! $post->title !!}
@endsection

Makes it evident that the content is not escaped, but @section('title', $post->title) doesn't.

@taylorotwell taylorotwell merged commit 98555a4 into laravel:5.4 Jan 20, 2017
@tillkruss
Copy link
Contributor

tillkruss commented Jan 22, 2017

Nice one. @taylorotwell This should probably be mentioned in the 5.4 upgrade guide, right?

@maddhatter
Copy link
Contributor

maddhatter commented Jan 23, 2017

This broke a couple of sections for me - something in the upgrade guide would be awesome.

@sileence
Copy link
Contributor Author

@maddhatter it's easier to notice HTML being escaped than the other way around.

@maddhatter
Copy link
Contributor

@sileence Yeah, I totally get the motivation for the change and agree with it. It just took awhile of working my way through the all the Blade classes/methods to find what had changed so I could fix my code.

@ChristopheB
Copy link
Contributor

I'm having an issue with this change. I was using the method inject to dynamically fill a section with another view, something like this:

$view = \View::make('layout');
$view->getFactory()->inject('section', $anotherView->render());

I can't do this anymore, as now the injected view gets encoded by e(). Here is the awkward workaround I'm currently using:

$view = \View::make('layout');
$view->getFactory()->startSection('section');
echo $anotherView->render();
$view->getFactory()->stopSection();

I don't know what was the exact purpose of the method inject, but I would say I try to use it the intended way (?). This would fix the issue I have:

public function inject($section, $content)
{
    return $this->extendSection($section, $content);
}

If that's ok, I can make a PR with this change.

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Feb 17, 2017

The correct work around is to just use the HtmlString class (at least, the best way I can think of).

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Feb 17, 2017

$view->getFactory()->inject('section', new HtmlString($anotherView->render()));

@ChristopheB
Copy link
Contributor

Nice one Graham, I'll use that, thanks! 👍

@GrahamCampbell
Copy link
Member

We probably need better docs for this tbh. :)

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

Successfully merging this pull request may close these issues.

6 participants