Harden TwigSandbox methods

Created on 24 September 2021, almost 3 years ago
Updated 3 June 2024, 22 days ago

Problem/Motivation

When developing โœจ Add $entity->toUrl() and $entity->toLink() methods to allowed methods list in Twig sandbox policy Needs work it was identified that the current sandbox method matching is very broad, allowing for methods to match _any_ object. This means it could possibly lead to unintended calls to possibly unsafe methods.

By hardening this is also makes rationalizing new additions to the allowed methods like ::toUrl and ::toLink if its possible to restrict the methods to a specific interface instead of trying to guess at any place the method might exist and how safe or unsafe its usage might be.

Steps to reproduce

Proposed resolution

Modify the format of the allowed_method settings to allow targeting methods on specific interfaces or classes. Something like:

    $allowed_methods = Settings::get('twig_sandbox_allowed_methods', [
      // Only allow idempotent methods.
      EntityInterface::class . '::id',
      EntityInterface::class . '::label',
      EntityInterface::class . '::bundle',
      // Globally allowed methods.
      '::get',
      '::__toString',
      '::toString',
    ]);

Remaining tasks

Finalize how much we harden this.

User interface changes

n/a

API changes

Data model changes

Release notes snippet

TODO and needed.

๐Ÿ“Œ Task
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Themeย  โ†’

Last updated about 4 hours ago

Created by

๐Ÿ‡บ๐Ÿ‡ธUnited States neclimdul Houston, TX

Live updates comments and jobs are added and updated live.
  • Security

    It is used for security vulnerabilities which do not need a security advisory. For example, security issues in projects which do not have security advisory coverage, or forward-porting a change already disclosed in a security advisory. See Drupalโ€™s security advisory policy for details. Be careful publicly disclosing security vulnerabilities! Use the โ€œReport a security vulnerabilityโ€ link in the project pageโ€™s sidebar. See how to report a security issue for details.

  • Performance

    It affects performance. It is often combined with the Needs profiling tag.

  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Production build 0.69.0 2024