mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(mapping): a PHP method with a visibility modifier is a region
`^\s*function` matched a bare declaration and nothing else, so every method
written the way modern PHP writes them -- `public function`, `protected
function`, `private static function` -- was invisible to the region model.
The effect is not subtle on class-based plugins. MW WP Form's
`class.mail.php` produced ONE file-level region spanning 272 lines instead
of 16 method-scoped ones, and its `protected function _delete_files()` --
which holds CVE-2023-6559 -- was not a region at all. Paid Memberships Pro
happened to work only because that particular file omits the modifiers.
class.mail.php 1 file-level region -> 16 method-scoped regions
Also adds the MW WP Form checkout as the PATH family's plugin, pinned at
3bdd33d (5.0.3) with CVE-2023-6559 verified in the code:
protected function _delete_files() {
foreach ( $this->attachments as $file ) {
if ( file_exists( $file ) ) {
unlink( $file );
and the fix at 02ebc67 replacing the existence check with `realpath()`,
`is_file()` and a directory-confinement test. `unlink` is a modelled path
sink and `realpath` a modelled sanitizer, so this is a pair the engine
should separate rather than merely flag -- which is what makes it worth
more than a single hit.
Gates byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
This commit is contained in:
co-authored by
Claude Opus 5
parent
f13934c663
commit
37a6594703
@@ -0,0 +1,36 @@
|
||||
# MW WP Form 5.0.3 -- a WordPress plugin with an arbitrary file deletion.
|
||||
#
|
||||
# The PATH family's checkout, chosen so the class-by-class table (5.8) has more than one flaw class in it.
|
||||
# Verified in the code: at this commit `classes/services/class.mail.php:259` reads
|
||||
#
|
||||
# protected function _delete_files() {
|
||||
# foreach ( $this->attachments as $file ) {
|
||||
# if ( file_exists( $file ) ) {
|
||||
# unlink( $file );
|
||||
#
|
||||
# and the fix (02ebc67, "fix Delete only attachments in MW_WP_Form_Mail_Service->_delete_files()") replaces
|
||||
# the existence check with `realpath()`, `is_file()` and a confinement test against the plugin's own upload
|
||||
# directory. `unlink` is a modelled path sink and `realpath` a modelled sanitizer, so this is a pair the
|
||||
# engine should be able to separate rather than merely flag.
|
||||
|
||||
name = "mwwpform"
|
||||
url = "https://github.com/inc2734/mw-wp-form.git"
|
||||
commit = "3bdd33d834afe449c898b78ed6f71a57193df71b"
|
||||
language = "php"
|
||||
license = "GPL-2.0"
|
||||
license_file = "readme.txt"
|
||||
measures = ["detection"]
|
||||
note = """
|
||||
The parent of the fix commit; released as 5.0.3, fixed in 5.0.4 (2023-12-08). Nothing is vendored -- the
|
||||
recipe fetches on request and CI never does.
|
||||
"""
|
||||
|
||||
[[known]]
|
||||
id = "CVE-2023-6559"
|
||||
family = "path"
|
||||
weakness = "arbitrary file deletion: an attachment path reaches unlink() behind a file_exists check that confines nothing"
|
||||
file = "classes/services/class.mail.php"
|
||||
function = "_delete_files"
|
||||
line = 259
|
||||
reference = "fixed in 02ebc67b5856787e25c357ce928b61f8e37920f4, which adds realpath(), is_file() and a directory-confinement test; the vulnerable line is verified in the checkout"
|
||||
in_scope = true
|
||||
@@ -218,7 +218,13 @@ _PHP_REGISTRATION = re.compile(
|
||||
_PHP_REST_ROUTE = re.compile(r"\bregister_rest_route\s*\(")
|
||||
_PHP_CALLBACK = re.compile(r"""['"]callback['"]\s*=>\s*""" + _PHP_CALLABLE)
|
||||
_PHP_PERMISSION = re.compile(r"""['"]permission_callback['"]\s*=>\s*(?P<value>[^,\)]+)""")
|
||||
_PHP_FUNCTION = re.compile(r"^\s*function\s+(?P<name>[A-Za-z_][A-Za-z0-9_]*)\s*\(")
|
||||
# A method carries visibility and modifiers before `function`, and modern PHP always writes them. Matching
|
||||
# only a bare `function` made every class-based plugin ONE file-level region: MW WP Form's
|
||||
# `protected function _delete_files()` was invisible, while Paid Memberships Pro's methods happened to
|
||||
# work because that file omits the modifiers.
|
||||
_PHP_FUNCTION = re.compile(
|
||||
r"^\s*(?:(?:final|abstract|public|protected|private|static)\s+)*function\s+&?\s*(?P<name>[A-Za-z_][A-Za-z0-9_]*)\s*\("
|
||||
)
|
||||
|
||||
|
||||
def _php_entry_points(target: FileTarget, text: str) -> list[EntryPointRecord]:
|
||||
|
||||
@@ -332,3 +332,30 @@ def test_an_array_callable_is_a_handler_too(tmp_path: Path) -> None:
|
||||
assert by_name["get_order"].kind == "route", "an array callable registers a route"
|
||||
assert by_name["get_order"].access_level == "authenticated", "its permission_callback is not __return_true"
|
||||
assert by_name["fetch"].access_level == "public", "nopriv, through an array callable"
|
||||
|
||||
|
||||
def test_a_php_method_with_a_visibility_modifier_is_a_region(tmp_path: Path) -> None:
|
||||
"""Modern PHP always writes `public`/`protected`/`private` before `function`.
|
||||
|
||||
Matching only a bare `function` made every class-based plugin ONE file-level region: MW WP Form's
|
||||
`protected function _delete_files()` was invisible, and its file went from 16 method-scoped regions to a
|
||||
single one spanning everything. Paid Memberships Pro happened to work only because that file omits the
|
||||
modifiers.
|
||||
"""
|
||||
from openultrasast.mapping import analyze_entry_points
|
||||
from openultrasast.preprocess import preprocess_repository
|
||||
|
||||
(tmp_path / "svc.php").write_text(
|
||||
"<?php\n"
|
||||
"class Service {\n"
|
||||
" public function open($p) { return fopen($p, 'r'); }\n"
|
||||
" protected function _delete_files($f) { unlink($f); }\n"
|
||||
" private static function helper($x) { return $x; }\n"
|
||||
" function bare($y) { echo $y; }\n"
|
||||
"}\n"
|
||||
)
|
||||
_, targets = preprocess_repository(tmp_path)
|
||||
|
||||
names = {entry.function_name for entry in analyze_entry_points(tmp_path, targets)}
|
||||
|
||||
assert {"open", "_delete_files", "helper", "bare"} <= names
|
||||
|
||||
Reference in New Issue
Block a user