search

rcrowe / TwigBridge

Give the power of Twig to Laravel
MIT License
894 stars 168 forks source link

2.x compatibility updates broke Eloquent Model sandbox patch #362

Closed uphlewis closed 5 years ago

uphlewis commented 5 years ago

This commit was designed to fix the sandbox bypass issue on eloquent models (because they implement ArrayAccess) https://github.com/rcrowe/TwigBridge/pull/266

That method has since become obsolete with the 2.x combatability changes, and was since removed in https://github.com/rcrowe/TwigBridge/pull/355

Is there an alternative workaround now that Template::getAttribute() is now obselete ??

barryvdh commented 5 years ago

Hmm, not really sure.

uphlewis commented 5 years ago

Obviously its only really an issue for user-supplied templates, because eloquent models can easily be traversed via relations, and queries can be called through the ArrayAccess interfaced methods. (e.g., {{model.save}} actually calls the underlying $model->save() method via $model['save'], despite then throwing an exception).

Because Twig treats ArrayAccess objects as arrays, it bypasses the SecurityPolicy's allowedMethods (which kinda makes sense, but is still scary when you consider Eloquent Models).

In my own implementation, before rendering I "filter" out any ArrayAccess objects from the context array before they get passed to twig.

A simple recursive function that tries to invoke ->toArray() or otherwise cast as (array), then do the same on the resulting array.

This would be a breaking change however, so perhaps could be added as a mode in the configuration?

bilogic commented 5 years ago

Same, I had to downgrade twig from 2.7.0 to 2.6.2 to work

bytestream commented 5 years ago

Pushed a fix for this which should operate the same as the fix I pushed for v1 originally.

Also linking this to https://github.com/twigphp/Twig/issues/2878 - I don't think this is an issue unless you explicitly allow a property 'save' in the security policy