From 98359d4cfbc78ad303868f2e58c1c5a705a8c27a Mon Sep 17 00:00:00 2001 From: magdev Date: Sun, 1 Mar 2026 01:02:43 +0100 Subject: [PATCH] Security audit fixes: fn() whitelist, escaping, and performance (v0.1.4) - WooCommerceExtension: ALLOWED_FUNCTIONS whitelist for fn() Twig function - Notice templates: data attributes use wp_kses_post instead of raw - Search form: esc_attr on search query value attribute - Per-request ContextBuilder caching via static variable - Shared wc_bootstrap_render_in_page_shell() helper (DRY) - Removed unused WC_BOOTSTRAP_VERSION and WC_BOOTSTRAP_URL constants Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 17 ++++ CLAUDE.md | 21 ++++- functions.php | 109 ++++++++++++------------- inc/Twig/WooCommerceExtension.php | 26 +++++- style.css | 2 +- templates/notices/error.html.twig | 2 +- templates/notices/notice.html.twig | 2 +- templates/notices/success.html.twig | 2 +- templates/product-searchform.html.twig | 2 +- 9 files changed, 118 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f873366..47651a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ All notable changes to this project will be documented in this file. +## [0.1.4] - 2026-03-01 + +### Security + +- **fn() function whitelist** (`WooCommerceExtension`): The `callFunction()` method (exposed as `fn()` in Twig templates) now restricts callable functions to an explicit `ALLOWED_FUNCTIONS` whitelist. Previously any PHP function could be called, risking arbitrary code execution if template context were compromised. Only the 6 functions actually used in templates are permitted. +- **Notice data attribute escaping**: Changed `{{ notice.data|raw }}` to `{{ notice.data|wp_kses_post }}` in success, error, and notice Twig templates. Defense-in-depth against potential XSS via data attributes. +- **Search query escaping** (`product-searchform.html.twig`): Added `|esc_attr` filter to `get_search_query()` output in the search input value attribute. + +### Performance + +- **Per-request ContextBuilder caching**: New `wc_bootstrap_get_theme_context()` function with static variable caching eliminates redundant `ContextBuilder::build()` calls (10-20 DB queries each) when multiple WooCommerce render functions fire in the same request. + +### Changed + +- **Shared page shell helper**: New `wc_bootstrap_render_in_page_shell()` function extracts the duplicated context-injection-and-render pattern from `wc_bootstrap_render_page()`, `wc_bootstrap_render_product_archive()`, and `wc_bootstrap_render_single_product()`. +- **Removed unused constants**: Removed `WC_BOOTSTRAP_VERSION` and `WC_BOOTSTRAP_URL` constants that were defined but never referenced. + ## [0.1.3] - 2026-02-28 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 6af3db0..406127c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -327,10 +327,29 @@ The child theme inherits from `wp-bootstrap` via WordPress `Template: wp-bootstr ## Version History -Current version: **v0.1.3** +Current version: **v0.1.4** ## Session History +### 2026-03-01 — v0.1.4 Security Audit & Performance Fixes + +**Scope:** Cross-theme security audit (12 findings), all fixed. Covers fn() whitelist, notice data escaping, search query escaping, per-request context caching, shared render helper, and unused constant removal. + +**Files changed (6):** + +- `inc/Twig/WooCommerceExtension.php` — Added `ALLOWED_FUNCTIONS` whitelist to `callFunction()`. Only 6 functions (`WC`, `_n`, `get_pagenum_link`, `wc_review_ratings_enabled`, `wc_get_product_category_list`, `wc_get_product_tag_list`) are permitted. +- `templates/notices/success.html.twig` — `notice.data|raw` → `notice.data|wp_kses_post` +- `templates/notices/error.html.twig` — `notice.data|raw` → `notice.data|wp_kses_post` +- `templates/notices/notice.html.twig` — `notice.data|raw` → `notice.data|wp_kses_post` +- `templates/product-searchform.html.twig` — Added `|esc_attr` on `get_search_query()` value +- `functions.php` — Removed unused `WC_BOOTSTRAP_VERSION`/`WC_BOOTSTRAP_URL` constants. Added `wc_bootstrap_get_theme_context()` with static caching and `wc_bootstrap_render_in_page_shell()` helper. Refactored 3 render functions to use shared helpers. + +**Key decisions:** + +- **fn() whitelist defense-in-depth**: Template files are static PHP, not user-editable, so exploitation requires file write access. Whitelist added anyway as defense-in-depth to prevent `exec()`, `system()`, etc. if template context were ever compromised. +- **`|wp_kses_post` over `|raw` for data attributes**: WooCommerce sanitizes notice data, but belt-and-suspenders approach prevents XSS if upstream behavior changes. +- **Static variable caching over transients**: Per-request `static $cached_context` is sufficient since WooCommerce pages build context once. No transient overhead or invalidation needed. + ### 2026-02-28 — My Account Bootstrap 5 Polish **Scope:** Redesigned 8 my-account Twig templates + CSS overrides to feel like a polished Bootstrap 5 application. diff --git a/functions.php b/functions.php index f607044..8d6a314 100644 --- a/functions.php +++ b/functions.php @@ -17,15 +17,8 @@ if ( ! defined( 'ABSPATH' ) ) { /** * Define theme constants. - * - * CRITICAL: WordPress reads the version from TWO places: - * 1. style.css header "Version:" — WordPress uses THIS for admin display - * 2. This PHP constant — used internally by the theme - * Both MUST be updated on every release. */ -define( 'WC_BOOTSTRAP_VERSION', '0.1.0' ); define( 'WC_BOOTSTRAP_PATH', get_stylesheet_directory() . '/' ); -define( 'WC_BOOTSTRAP_URL', get_stylesheet_directory_uri() . '/' ); /** * Load Composer autoloader if present. @@ -140,6 +133,54 @@ function wc_bootstrap_enqueue_scripts(): void { } add_action( 'wp_enqueue_scripts', 'wc_bootstrap_enqueue_scripts' ); +/** + * Build the parent theme context for a page render. + * + * Caches the ContextBuilder result per request to avoid redundant database + * queries when multiple WooCommerce rendering functions need the same context. + * + * @since 0.1.1 + * + * @return array Theme context array. + */ +function wc_bootstrap_get_theme_context(): array { + static $cached_context = null; + + if ( null === $cached_context ) { + $context_builder = new \WPBootstrap\Template\ContextBuilder(); + $cached_context = $context_builder->build(); + } + + return $cached_context; +} + +/** + * Render content inside the parent theme's page shell. + * + * Injects the given HTML content into the parent theme's page template, + * replacing the post content. Title and thumbnail are blanked so the + * parent theme does not render its own headings — the content handles that. + * + * @since 0.1.1 + * + * @param string $content HTML content to render inside the page shell. + */ +function wc_bootstrap_render_in_page_shell( string $content ): void { + $theme_context = wc_bootstrap_get_theme_context(); + $twig = \WPBootstrap\Twig\TwigService::getInstance(); + + $theme_context['post'] = array_merge( + $theme_context['post'] ?? [], + [ + 'content' => $content, + 'title' => '', + 'thumbnail' => '', + ] + ); + + echo $twig->render( 'pages/page.html.twig', $theme_context ); +} + /** * Handle plugin page rendering via plugin render filter. * @@ -157,26 +198,10 @@ add_action( 'wp_enqueue_scripts', 'wc_bootstrap_enqueue_scripts' ); function wc_bootstrap_render_page( bool $rendered, string $content, array $context ): bool { if ( ! class_exists( '\WPBootstrap\Twig\TwigService' ) || ! class_exists( '\WPBootstrap\Template\ContextBuilder' ) ) { - return false; // Can't render, let plugin use its own fallback + return false; } - $context_builder = new \WPBootstrap\Template\ContextBuilder(); - $theme_context = $context_builder->build(); - $twig = \WPBootstrap\Twig\TwigService::getInstance(); - - // Inject plugin content as the page post content so page.html.twig renders it - // inside the standard content block. Title is empty so the parent theme does not - // render its own

— plugin templates handle their own headings. - $theme_context['post'] = array_merge( - $theme_context['post'] ?? [], - [ - 'content' => $content, - 'title' => '', - 'thumbnail' => '', - ] - ); - - echo $twig->render( 'pages/page.html.twig', $theme_context ); + wc_bootstrap_render_in_page_shell( $content ); return true; } add_filter( 'woocommerce_render_page', 'wc_bootstrap_render_page', 10, 3 ); @@ -359,26 +384,11 @@ function wc_bootstrap_render_product_archive(): void { return; } - // Capture WooCommerce archive content via output buffering. ob_start(); include get_stylesheet_directory() . '/woocommerce/archive-product.php'; $content = ob_get_clean(); - // Build parent theme context and inject archive content into page shell. - $context_builder = new \WPBootstrap\Template\ContextBuilder(); - $theme_context = $context_builder->build(); - $twig = \WPBootstrap\Twig\TwigService::getInstance(); - - $theme_context['post'] = array_merge( - $theme_context['post'] ?? [], - [ - 'content' => $content, - 'title' => '', - 'thumbnail' => '', - ] - ); - - echo $twig->render( 'pages/page.html.twig', $theme_context ); + wc_bootstrap_render_in_page_shell( $content ); exit; } add_action( 'template_redirect', 'wc_bootstrap_render_product_archive', 11 ); @@ -406,26 +416,11 @@ function wc_bootstrap_render_single_product(): void { return; } - // Capture WooCommerce single product content via output buffering. ob_start(); include get_stylesheet_directory() . '/woocommerce/single-product.php'; $content = ob_get_clean(); - // Build parent theme context and inject product content into page shell. - $context_builder = new \WPBootstrap\Template\ContextBuilder(); - $theme_context = $context_builder->build(); - $twig = \WPBootstrap\Twig\TwigService::getInstance(); - - $theme_context['post'] = array_merge( - $theme_context['post'] ?? [], - [ - 'content' => $content, - 'title' => '', - 'thumbnail' => '', - ] - ); - - echo $twig->render( 'pages/page.html.twig', $theme_context ); + wc_bootstrap_render_in_page_shell( $content ); exit; } add_action( 'template_redirect', 'wc_bootstrap_render_single_product', 11 ); diff --git a/inc/Twig/WooCommerceExtension.php b/inc/Twig/WooCommerceExtension.php index c9a8069..02913a5 100644 --- a/inc/Twig/WooCommerceExtension.php +++ b/inc/Twig/WooCommerceExtension.php @@ -235,18 +235,40 @@ class WooCommerceExtension extends AbstractExtension { } /** - * Call a PHP function by name and return its result. + * Allowlist of PHP functions that can be called via fn() in Twig templates. + * + * Prevents arbitrary function execution (e.g., exec, system) if template + * context were ever compromised. Only functions actually used in templates + * are permitted. + */ + private const ALLOWED_FUNCTIONS = [ + 'WC', + '_n', + 'get_pagenum_link', + 'wc_review_ratings_enabled', + 'wc_get_product_category_list', + 'wc_get_product_tag_list', + ]; + + /** + * Call a whitelisted PHP function by name and return its result. * * Enables `fn('WC')` in templates to access the WooCommerce singleton * and chain method calls via Twig's property accessor. * + * Only functions in the ALLOWED_FUNCTIONS list can be called. This prevents + * arbitrary code execution if template context were ever compromised. + * * @param string $name Function name. * @param mixed ...$args Arguments. * @return mixed Function return value. * - * @throws \RuntimeException If function does not exist. + * @throws \RuntimeException If function is not allowed or does not exist. */ public function callFunction( string $name, ...$args ): mixed { + if ( ! in_array( $name, self::ALLOWED_FUNCTIONS, true ) ) { + throw new \RuntimeException( "Function {$name} is not allowed. Add it to ALLOWED_FUNCTIONS." ); + } if ( ! function_exists( $name ) ) { throw new \RuntimeException( "Function {$name} does not exist." ); } diff --git a/style.css b/style.css index e0f44f7..d41cec3 100644 --- a/style.css +++ b/style.css @@ -7,7 +7,7 @@ Description: A Bootstrap 5 child theme for WP Bootstrap that overrides all WooCo Requires at least: 6.7 Tested up to: 6.7 Requires PHP: 8.3 -Version: 0.1.3 +Version: 0.1.4 License: GNU General Public License v2 or later License URI: http://www.gnu.org/licenses/gpl-2.0.html Template: wp-bootstrap diff --git a/templates/notices/error.html.twig b/templates/notices/error.html.twig index 2aca260..8f723dd 100644 --- a/templates/notices/error.html.twig +++ b/templates/notices/error.html.twig @@ -23,7 +23,7 @@ {% else %}
    {% for notice in notices %} -
  • +
  • {{ notice.notice|raw }}
    • {% endfor %} diff --git a/templates/notices/notice.html.twig b/templates/notices/notice.html.twig index 132ebde..d4a9fbf 100644 --- a/templates/notices/notice.html.twig +++ b/templates/notices/notice.html.twig @@ -17,7 +17,7 @@ {% if notices is defined and notices|length > 0 %} {% for notice in notices %}
    {{ notice.notice|raw }} diff --git a/templates/notices/success.html.twig b/templates/notices/success.html.twig index 938ad46..b0dad57 100644 --- a/templates/notices/success.html.twig +++ b/templates/notices/success.html.twig @@ -17,7 +17,7 @@ {% if notices is defined and notices|length > 0 %} {% for notice in notices %}