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.1] [5.3] @parent tag is not escaping #10068

Closed
abhimanyu003 opened this issue Aug 28, 2015 · 25 comments
Closed

[5.1] [5.3] @parent tag is not escaping #10068

abhimanyu003 opened this issue Aug 28, 2015 · 25 comments
Labels

Comments

@abhimanyu003
Copy link
Contributor

abhimanyu003 commented Aug 28, 2015

@parent is not escaping when the data comes from the MySQL but it get's compiled and executed. This can be a security issue.

I have created a sample repo to demonstrate the issue https://github.com/abhimanyu003/laravelSample

  • Clone the repo and run composer install
  • Run php artisan migrate
  • Run php aritsan db:seed ( Which will create a user with name @parent )
  • Visit home page

You will see a notice Master sidebar only display if @\parent tag is executed. which means that @parent tag is executed and not escaped out when it is getting data from MySQL.

I'm just fetching the first user from MySQL in routes.php file https://github.com/abhimanyu003/laravelSample/blob/master/app/Http/routes.php then that data passed to view child ( Location: resouces/views/child.blade.php )

You can see at line no 4. {{ $user->name }} https://github.com/abhimanyu003/laravelSample/blob/master/resources/views/child.blade.php#L4 this is the place where blade supposed to escape @parent tag but it is not rather it is excution it and appending it master.blade.php

Expected Result:

    @parent
    <p>This is appended to the master sidebar.</p>

Current Result:

Master sidebar only display if @\parent tag is executed.
This is appended to the master sidebar.

PS: @parent is the also the name of user in database.

Thanks

@GrahamCampbell
Copy link
Member

You should NEVER execute php code from user input. That's what you're doing here. Looking at your code there, I don't see why you have an issue though, because you're just echoing a value in php.

@abhimanyu003
Copy link
Contributor Author

How come @parent is php code ? It a blade syntax.

@GrahamCampbell
Copy link
Member

Because it gets compiled by blade into php, then the file is "required".

@GrahamCampbell
Copy link
Member

From your code sample there, that's not happening though.

@GrahamCampbell
Copy link
Member

That sort of thing would only happen if you're actually building files from user input.

@abhimanyu003
Copy link
Contributor Author

@GrahamCampbell I'm doing nothing special you can see, but somehow @parent is executing. Let's say if there is some textarea on my website and i'm displaying in the same way. It can break the current website.

So can you please tell how to avoid so that @parent will not get executed.

@abhimanyu003 abhimanyu003 changed the title @parent tag is escaping @parent tag is not escaping Aug 28, 2015
@GrahamCampbell
Copy link
Member

I'm doing nothing special you can see, but somehow @parent is executing.

That's impossible from that code sample.

@GrahamCampbell
Copy link
Member

We don't even have a way to execute @parent. It has to be compiled first then the result must be required.

@GrahamCampbell
Copy link
Member

So sorry. Just read our code again. It seems that part isn't done at the "compile" level, but is done after.

@GrahamCampbell
Copy link
Member

You have indeed uncovered a security issue. I'm going to contact Taylor.

@abhimanyu003
Copy link
Contributor Author

@GrahamCampbell many many thanks for reopening the issue. :)

@GrahamCampbell
Copy link
Member

Not at all, security is very important. Sorry I took a while to realise what was going on here.

@GrahamCampbell
Copy link
Member

In future, could you please contact Taylor directly regarding security issues, because now this is public knowledge to anyone wanting to target Laravel apps.

@GrahamCampbell
Copy link
Member

IMO, the issue here is not so much the escaping, but more, that there's an inherit design floor here that allows this to happen in the first place.

@abhimanyu003
Copy link
Contributor Author

Ok, so sorry for not contacting Taylor directly and I will make sure of this next time. I will wait for some update.

@laurencei
Copy link
Contributor

I reported this exact issue months ago - and it got closed to due inactivity: #7888

@abhimanyu003
Copy link
Contributor Author

@theshiftexchange this so strange, I'm giving try to few things but i'm not sure what causing this. I can't say much but this need to resolved quickly.

@GrahamCampbell GrahamCampbell reopened this Sep 4, 2015
mcgrogan91 added a commit to mcgrogan91/framework that referenced this issue Oct 2, 2015
mcgrogan91 added a commit to mcgrogan91/framework that referenced this issue Oct 2, 2015
It's probably safer to assume e is a function that was defined before, and is not guaranteed to be the one we see above us in the code.

Fixing PSR

StyleCI fix

IDE put spaces there, and there shouldn't be any
mradlinski added a commit to mradlinski/laravel that referenced this issue Nov 8, 2015
@GrahamCampbell GrahamCampbell changed the title @parent tag is not escaping [5.1] [5.2] @parent tag is not escaping Dec 20, 2015
@GrahamCampbell
Copy link
Member

Is anyone able to look into this? The previous fix we had was very good, but it had to be reverted due to a bug.

@Ruby184
Copy link
Contributor

Ruby184 commented Dec 27, 2015

I looked into this and put something together, just need some tests and i will send a PR. @GrahamCampbell should I send a PR to 5.1 or 5.2 branch ? Thanks

@halaei
Copy link
Contributor

halaei commented Jun 22, 2016

Sorry to disturb, but can somebody fix it for 5.3 at least? I only know how to remove the feature, if you like me to submit a PR!? I am sending this just because 5.3 is coming soon and I think it is better not to have this bug there, than be backward compatible.

@GrahamCampbell
Copy link
Member

The PR had to be reverted not because of BC, but because it didn't actually work properly. We'd definitely accept a PR that does work. :)

@taylorotwell
Copy link
Member

Yes it should be fixed, not removed.

@abhimanyu003
Copy link
Contributor Author

@taylorotwell can I request re-open for community to help ?

@taylorotwell taylorotwell reopened this Jun 22, 2016
@scazzy
Copy link

scazzy commented Jul 12, 2016

For now, since we don't use the @parent feature of blade, we've overwritten the method and commented the str_replace line.

@themsaid themsaid changed the title [5.1] [5.2] @parent tag is not escaping [5.1] [5.3] @parent tag is not escaping Sep 28, 2016
@themsaid
Copy link
Member

this was fixed in #16033

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

No branches or pull requests

8 participants