Skip to content

Backport view path containment check for CVE-2026-48820 - #22

Merged
basi merged 4 commits into
mainfrom
work/fix-view-path-traversal-cve-2026-48820
Sep 3, 2026
Merged

basi merged 4 commits into
mainfrom
work/fix-view-path-traversal-cve-2026-48820

Conversation

@basi

@basi basi commented Aug 26, 2026 •

Copy link
Copy Markdown
Owner

Problem

CVE-2026-48820 / GHSA-wpvj-hjcr-h3p2 allows a request-controlled element name containing ../ to make CakePHP include a PHP template outside the configured view template paths. CakePHP fixed the issue in 4.5.11 with cakephp/cakephp@2af17bd02, but CakePHP 2.x is EOL and requires a backport.

This fork is used by bask/cakephp in production, so leaving Dependabot alert #6 unresolved exposes applications that pass request data into view resolution to local file inclusion of .ctp files.

Changes

  • Add View::_checkFilePath() with a no-.. fast path and canonical containment checks against all configured view template roots.
  • Apply the check to view, layout, and element resolution, plus the ScaffoldView override.
  • Add six regression tests covering element, layout, and both vulnerable view-name rewrite paths, including valid relative paths that remain inside a view root.
  • Document the security backport and its elementExists() compatibility note in the changelog.
  • Prepare the automated release workflow for v1.2.0 because the new exception behavior is a minor-version compatibility change.

Deliberate deviation from upstream

The upstream implementation compares the canonical candidate path with raw configured roots. This backport also compares it with realpath() of each root and normalizes the trailing directory separator.

The additional comparison preserves valid ../ templates in deployments whose configured view root contains a symlink, such as current -> releases/N. It is additive: a path is accepted only when its canonical target is actually contained by a configured canonical root. The separator normalization also prevents prefix collisions such as /app/View and /app/ViewX.

Compatibility and BC notes

  • Normal template resolution has no added realpath() cost because paths without .. return immediately.
  • The raw candidate path is returned after validation, preserving parent-view and element-cache keys.
  • bask/cakephp has no .. view/element calls, elementExists() calls, custom view roots, or themes, so no consumer impact is expected.
  • elementExists() now throws InvalidArgumentException for an existing template that resolves outside all view roots instead of returning false. This matches CakePHP 4.5.11.
  • Accepted residual risks match upstream: names without .. can follow an outward symlink inside a view root; realpath() failure is passed through; and a filesystem race remains possible between validation and inclusion.

Test plan

  • Confirmed before applying the implementation that the new escape tests reached or included the out-of-root core template instead of throwing InvalidArgumentException.
  • git diff --check
  • PHP 8.2 syntax checks for lib/Cake/View/View.php and lib/Cake/View/ScaffoldView.php
  • Focused regression suite: 6 tests, 10 assertions — pass
  • Complete ViewTest: 87 tests, 172 assertions — pass with 5 pre-existing PHPUnit deprecation warnings
  • Fable read-only security/diff audit — no blocking findings
  • Full AllTestsTest was attempted and stopped at 1,952 / 4,078 tests on the unrelated existing PHP 8.2 deprecation Creation of dynamic property FormHelperTest::$db is deprecated.
  • PHPCS was attempted, but the repository-pinned PHPCS crashes on PHP 8.2 before analyzing files because it uses removed curly-brace string-offset syntax.

GitHub Actions remains the final matrix gate for PHP 8.0/8.2 with MySQL, PostgreSQL, and SQLite.

Post-merge steps

  1. Let the main push trigger bump-version.yml to create the v1.2.0 tag and release, with the elementExists() BC note in the release notes.
  2. Dismiss Dependabot alert Add RedisCluster cache engine with PHP 8 support #6 with a comment linking this PR because the forked Composer package version will not close it automatically.
  3. In bask/cakephp, update basi/cakephp2-php8 and verify that lib/Cake/View/View.php is refreshed in the dependency-copy diff.
  4. Optionally offer the same backport to kamilwylegala/cakephp2-php8.

Reject view, layout, element, and scaffold template paths that resolve
outside configured view roots.

Add regression coverage and prepare the v1.2.0 release.
@basi

basi commented Aug 26, 2026

Copy link
Copy Markdown
Owner Author

CI status note: all four matrix jobs stopped before running tests during Composer dependency resolution. Current Composer blocks the repository's existing cakephp/cakephp-codesniffer 1.x dependency because it requires PHPCS 1.x versions covered by PKSA-rdkp-vv9z-mjkg and PKSA-6vdd-n4sx-knhy. This is independent of the five-file CVE backport. Local PHP 8.2 verification completed successfully for both modified files (syntax), the six focused regression tests (10 assertions), and the complete ViewTest suite (87 tests, 172 assertions).

@basi
basi marked this pull request as ready for review August 28, 2026 04:39
@basi basi self-assigned this Aug 28, 2026

@shin2ro shin2ro left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントをしているのでご確認をお願いします 🙏

Comment thread lib/Cake/View/View.php Outdated
}
foreach ($this->_paths($plugin) as $path) {
if (strpos($absolute, rtrim($path, DS) . DS) === 0) {
return $file;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

絶対パスの検証をした後なので $absolute を返してもよさそうです!

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread lib/Cake/View/View.php Outdated
}
$root = realpath($path);
if ($root !== false && strpos($absolute, rtrim($root, DS) . DS) === 0) {
return $file;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

絶対パスの検証をした後なので $absolute を返してもよさそうです!

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@basi
basi requested a review from shin2ro September 3, 2026 12:19

@shin2ro shin2ro left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@basi
basi merged commit 6544826 into main Sep 3, 2026
3 of 4 checks passed
@basi
basi deleted the work/fix-view-path-traversal-cve-2026-48820 branch September 3, 2026 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants