diff --git a/src/openultrasast/mapping.py b/src/openultrasast/mapping.py index 5c21c35..3e14fe3 100644 --- a/src/openultrasast/mapping.py +++ b/src/openultrasast/mapping.py @@ -204,12 +204,19 @@ def _language_entry_points(target: FileTarget, text: str) -> list[EntryPointReco # `wp_ajax_nopriv_*` is admin-ajax for logged-OUT users and is therefore declared public, while `wp_ajax_*` # fires only for logged-in ones. That is a DECLARED access level, not a guess from an absent decorator, which # is the distinction task 2.8 turns on. +# A callback is a bare string OR an array callable. `array( $this, 'method' )` is the normal form in a +# class-based plugin -- Paid Memberships Pro registers every REST route that way -- and matching only the +# string form left its handlers ranked as plain functions, below the budget, so the entry point that carries +# CVE-2023-23488's source was never asked about. +_PHP_CALLABLE = ( + r"""(?:['"](?P[A-Za-z_][A-Za-z0-9_]*)['"]|(?:array\s*\(|\[)\s*\$this\s*,\s*['"](?P[A-Za-z_][A-Za-z0-9_]*)['"])""" +) _PHP_REGISTRATION = re.compile( - r"""\b(?Padd_action|add_filter|add_shortcode)\s*\(\s*['"](?P[^'"]+)['"]\s*,\s*['"](?P[A-Za-z_][A-Za-z0-9_]*)['"]""", + r"""\b(?Padd_action|add_filter|add_shortcode)\s*\(\s*['"](?P[^'"]+)['"]\s*,\s*""" + _PHP_CALLABLE, re.VERBOSE, ) _PHP_REST_ROUTE = re.compile(r"\bregister_rest_route\s*\(") -_PHP_CALLBACK = re.compile(r"""['"]callback['"]\s*=>\s*['"](?P[A-Za-z_][A-Za-z0-9_]*)['"]""") +_PHP_CALLBACK = re.compile(r"""['"]callback['"]\s*=>\s*""" + _PHP_CALLABLE) _PHP_PERMISSION = re.compile(r"""['"]permission_callback['"]\s*=>\s*(?P[^,\)]+)""") _PHP_FUNCTION = re.compile(r"^\s*function\s+(?P[A-Za-z_][A-Za-z0-9_]*)\s*\(") @@ -226,7 +233,10 @@ def _php_entry_points(target: FileTarget, text: str) -> list[EntryPointRecord]: for number, line in enumerate(lines, start=1): for match in _PHP_REGISTRATION.finditer(line): - hook, handler = match.group("hook"), match.group("handler") + hook = match.group("hook") + handler = match.group("handler") or match.group("method") + if not handler: + continue access, evidence = _wordpress_hook_access(match.group("call"), hook) start, end = bounds.get(handler, (number, number)) records.append( @@ -236,7 +246,9 @@ def _php_entry_points(target: FileTarget, text: str) -> list[EntryPointRecord]: window = "\n".join(lines[number - 1 : number + 12]) callback = _PHP_CALLBACK.search(window) if callback is not None: - handler = callback.group("handler") + handler = callback.group("handler") or callback.group("method") + if not handler: + continue permission = _PHP_PERMISSION.search(window) access, evidence = _rest_route_access(permission.group("value").strip() if permission else None) start, end = bounds.get(handler, (number, number)) diff --git a/tests/test_mapping.py b/tests/test_mapping.py index 5f930dc..ad946e9 100644 --- a/tests/test_mapping.py +++ b/tests/test_mapping.py @@ -302,3 +302,33 @@ def test_every_php_function_becomes_a_region(tmp_path: Path) -> None: regions = regions_for(analyze_entry_points(tmp_path, targets), targets) assert {region.function for region in regions} >= {"a", "b"} + + +def test_an_array_callable_is_a_handler_too(tmp_path: Path) -> None: + """`array( $this, 'method' )` is the normal form in a class-based plugin. + + Paid Memberships Pro registers every REST route that way, and matching only the string form left its + handlers ranked as plain functions below the scan budget -- so the entry point carrying CVE-2023-23488's + source was never asked about, while the sink two calls away was. + """ + from openultrasast.mapping import analyze_entry_points + from openultrasast.preprocess import preprocess_repository + + (tmp_path / "api.php").write_text( + " 'GET',\n" + " 'callback' => array( $this, 'get_order' ),\n" + " 'permission_callback' => array( $this, 'permissions_check' )\n" + ") ) );\n" + "add_action('wp_ajax_nopriv_fetch', array( $this, 'fetch' ));\n" + "function get_order($request) { echo 1; }\n" + "function fetch() { echo 2; }\n" + ) + _, targets = preprocess_repository(tmp_path) + + by_name = {entry.function_name: entry for entry in analyze_entry_points(tmp_path, targets)} + + 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"