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

[3.x, 4.x, 5.x]: Template::attribute() bypasses Twig SandboxExtension's SecurityPolicy #15278

Closed
khalwat opened this issue Jul 2, 2024 · 3 comments
Labels

Comments

@khalwat
Copy link
Contributor

khalwat commented Jul 2, 2024

What happened?

Description

The Template::attribute() method (which is what is called whenever accessing an object property in the PHP compiled from Twig) does the following:

        if (
            $type !== TwigTemplate::METHOD_CALL &&
            $object instanceof BaseObject &&
            $object->canGetProperty($item)
        ) {
            return $isDefinedTest ? true : $object->$item;
        }

This bypasses the call later in that function to twig_get_attribute() , which means that if the Twig SandboxExtension is used with a SecurityPolicy, the SecurityPolicy::checkPropertyAllowed() method is never called.

This makes it impossible to implement a SecurityPolicy that restricts access to properties on any object that is an instance of BaseObject (which just about everything in Craft is)

For instance, if you wanted to restrict access to:

{{ craft.app.config.db.password }}

...you can't do it, because the Twig layer that implements sandbox security checks is never called, because the DbConfig object inherits from BaseObject, so the property value is just returned.

Steps to reproduce

  1. Add the SandboxExtension to the Twig environment
  2. Use a SecurityPolicy that implements ::checkPropertyAllowed()
  3. Notice that it is never called when accessing an object property like entry.title on any BaseObject object

Expected behavior

It would be possible to use the SandboxExtension with a SecurityPolicy that restricts access to object properties.

This fixes the issue, allowing the SandboxExtension to optionally throw an exception before the property is returned:

        if (
            $type !== TwigTemplate::METHOD_CALL &&
            $object instanceof BaseObject &&
            $object->canGetProperty($item)
        ) {
            if ($sandboxed) {
                $env->getExtension(SandboxExtension::class)->checkPropertyAllowed($object, $item, $lineno, $source);
            }
            return $isDefinedTest ? true : $object->$item;
        }

This mirrors what twig_get_attribute() does.

Twig\Sandbox\SecurityNotAllowedPropertyError
Accessing "password" property on a "craft\config\DbConfig" object is not allowed in "__string_template__ac8af96bb981e782c298f5d7796307e3" at line 1.

Actual behavior

It's not possible :)

Craft CMS version

3.x, 4.x, 5.x

PHP version

n/a

Operating system and version

n/a

Database type and version

n/a

Image driver and version

n/a

Installed plugins and versions

n/a

@khalwat khalwat added the bug label Jul 2, 2024
@brandonkelly
Copy link
Member

Thanks for reporting! Fixed for the next Craft 4 and 5 releases.

@brandonkelly
Copy link
Member

4.10.4 and 5.2.5 are out with that fix.

@khalwat
Copy link
Contributor Author

khalwat commented Jul 3, 2024

@brandonkelly You are a gentleman and a scholar

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

2 participants