Commit 16486af
Make Twig sandbox object/function access policy configurable (blocklist + allowlist) (pimcore#19303)
* Make Twig sandbox object-access policy configurable (blocklist + allowlist)
SecurityPolicy previously enforced a single hardcoded denylist of
infrastructure classes for method/property access from sandboxed
templates. Add `templating_engine.twig.sandbox_security_policy`
options `blocked_classes` (extends the built-in denylist) and
`allowed_classes` (switches to allowlist mode, deactivating the
denylist entirely once non-empty), so sites can either extend the
denylist or lock the sandbox down to a known set of classes.
Related: GHSA-7gfm-v2fx-xrxm draft advisory recommends allowlisting
the sandbox's object access instead of relying solely on a denylist.
This change adds the mechanism; existing defaults are unchanged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Harden default sandbox object-access policy per GHSA-7gfm-v2fx-xrxm
Ship the advisory's minimum remediation as the default, not just as an
opt-in config:
- Add Pimcore\Model\User to the built-in denylist, so getPassword() and
getPasswordRecoveryToken() are no longer template-reachable out of
the box.
- Add an always-blocked-methods check (independent of blocklist/
allowlist mode) for Asset::getData()/getStream()/getLocalFile()/
getTemporaryFile(), so arbitrary asset content can't be exfiltrated
via a sandboxed template while Asset otherwise stays usable for
filename/thumbnail access. This hard block is not bypassed even if a
site explicitly allowlists User or Asset.
Extends the configurable blocked_classes/allowed_classes mechanism
added in the previous commit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Make the pimcore_* function auto-allow configurable (blocklist + allowlist)
Mirrors the blocked_classes/allowed_classes mechanism added for object
access: the pimcore_* prefix auto-allow in checkSecurity() now has its
own denylist/allowlist pair.
- blocked_functions extends the built-in denylist of pimcore_*
functions that look up and return a live model instance by id/path
(pimcore_user, pimcore_asset, pimcore_document, pimcore_site*, ...) -
consulted while allowed_functions is empty (default).
- allowed_functions, once non-empty, switches the pimcore_* prefix rule
to allowlist mode: the built-in + blocked_functions denylist is
deactivated entirely, and only the listed pimcore_* functions (plus
whatever is in the pre-existing `functions` allowlist) remain
callable.
The existing `functions`/$allowedFunctions option is unchanged - it
keeps working as an always-active explicit allowlist in both modes.
Default behavior (blocked_functions/allowed_functions both empty) is
identical to before this commit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Fix CI failure: use @dataProvider docblock, not PHPUnit attribute
Codeception's Unit test loader (Codeception\Test\DataProvider) only
resolves data providers via the @dataProvider docblock annotation - it
doesn't recognize PHPUnit\Framework\Attributes\DataProvider at all.
Using the attribute made Codeception run the two parameterized tests
with zero arguments, failing CI with ArgumentCountError. Verified the
fix directly against Codeception\Test\DataProvider::getDataForMethod.
Also addresses a review comment: replace the two live
`new PDO('sqlite::memory:')` connections with $this->createStub(PDO::class),
so the tests don't depend on the optional pdo_sqlite driver being
installed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Also hard-block User::getTwoFactorAuthentication() unconditionally
getTwoFactorAuthentication() (models/User.php:679-696) returns the
full 2FA config array, including the raw TOTP secret - a credential
equivalent to the password hash / recovery token. Allowlisting User
for otherwise-benign getters (e.g. getFirstname()) would still leak
the MFA secret via `pimcore_user(1).getTwoFactorAuthentication('secret')`,
defeating the point of allowlisting User at all.
Add it to ALWAYS_BLOCKED_METHODS alongside the two existing secret
getters, so it's hard-blocked regardless of blocklist/allowlist mode.
Test coverage and docs extended accordingly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Remove the pimcore_* function allowlist mode (allowed_functions)
Drop allowed_functions and the allowlist-mode switch it drove in
SecurityPolicy::isPimcoreFunctionAllowed() - the pimcore_* prefix rule
is now always denylist-only (built-in BLOCKED_FUNCTIONS + configured
blocked_functions), mirroring the hard-block approach already taken for
User::getTwoFactorAuthentication(). blocked_functions, allowed_classes/
blocked_classes and the general functions allowlist are unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Move built-in sandbox denylists from PHP constants to default.yaml
blocked_classes, blocked_functions and the FQCN=>methods map that hard-blocks
secret/content-returning getters (renamed always_blocked_methods ->
hard_blocked_methods) now carry their built-in values as the default in
bundles/CoreBundle/config/pimcore/default.yaml instead of private PHP
constants in SecurityPolicy. A site's own config for the same option is
merged with (appended to) that default rather than replacing it - verified
against Symfony's real Config/Processor component - so extending one of
these options cannot accidentally drop the shipped defaults.
SecurityPolicyTest's "*ByDefault" tests now read the actual shipped values
out of default.yaml instead of duplicating them in a PHP fixture.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Reduce the default pimcore_* blocked_functions denylist to pimcore_user
Comment out the id/path lookup functions (pimcore_asset, pimcore_document,
pimcore_object, pimcore_site and their variants) in the shipped
blocked_functions default, leaving only pimcore_user active by default.
These functions are auto-allowed by default now unless a site re-adds them.
Document the change: describe pimcore_user as the only function blocked
out of the box, list the now-auto-allowed id/path lookup functions and the
risk they carry (arbitrary id/path lookup exposing data outside a
template's intended scope), and recommend adding them back to
blocked_functions for a high-security setup - with the full list shown
both as the commented-out default.yaml entries and as a standalone example.
Note: this drops CI coverage - testIdLookupPimcoreFunctionsAreNotAutoAllowedByDefault
in SecurityPolicyTest reads the real blocked_functions default and will now
fail for 12 of its 13 cases (only the pimcore_user case still blocks).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Update SecurityPolicyTest to match the reduced blocked_functions default
testIdLookupPimcoreFunctionsAreNotAutoAllowedByDefault asserted all 13
id/path lookup functions are blocked by default, which no longer matches
default.yaml (only pimcore_user is still active there; the rest are
shipped commented out). Split it into:
- testPimcoreUserIsNotAutoAllowedByDefault - the one still blocked by
default.
- testOtherIdLookupPimcoreFunctionsAreAutoAllowedByDefault - confirms the
other 12 are now auto-allowed by default, matching the shipped config.
- testIdLookupPimcoreFunctionsCanBeBlockedForHighSecurity - confirms all
13 still block correctly once a site configures the full list, as
recommended in doc/26_Best_Practice/80_Twig_Sandbox_Object_Access.md.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>1 parent c5b2917 commit 16486af
9 files changed
Lines changed: 879 additions & 32 deletions
File tree
- bundles/CoreBundle
- config
- pimcore
- src/DependencyInjection
- doc
- 05_Objects/01_Object_Classes/03_Layout_Elements
- 19_Development_Tools_and_Details/25_Email_Framework
- 26_Best_Practice
- lib/Twig/Sandbox
- tests/Unit/Twig/Sandbox
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
336 | 336 | | |
337 | 337 | | |
338 | 338 | | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
339 | 373 | | |
340 | 374 | | |
341 | 375 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
80 | 84 | | |
81 | 85 | | |
82 | 86 | | |
| |||
Lines changed: 33 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2077 | 2077 | | |
2078 | 2078 | | |
2079 | 2079 | | |
| 2080 | + | |
| 2081 | + | |
| 2082 | + | |
| 2083 | + | |
| 2084 | + | |
| 2085 | + | |
| 2086 | + | |
| 2087 | + | |
| 2088 | + | |
| 2089 | + | |
| 2090 | + | |
| 2091 | + | |
| 2092 | + | |
| 2093 | + | |
| 2094 | + | |
| 2095 | + | |
| 2096 | + | |
| 2097 | + | |
| 2098 | + | |
| 2099 | + | |
| 2100 | + | |
| 2101 | + | |
| 2102 | + | |
| 2103 | + | |
| 2104 | + | |
| 2105 | + | |
| 2106 | + | |
| 2107 | + | |
| 2108 | + | |
| 2109 | + | |
| 2110 | + | |
| 2111 | + | |
| 2112 | + | |
2080 | 2113 | | |
2081 | 2114 | | |
2082 | 2115 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
80 | 80 | | |
81 | 81 | | |
82 | 82 | | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
83 | 87 | | |
84 | 88 | | |
85 | 89 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
Lines changed: 6 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
112 | 112 | | |
113 | 113 | | |
114 | 114 | | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
0 commit comments