Skip to content

Commit ed9862a

Browse files
committed
Calibrate wp_ajax nonce check to endpoint-level callback verification
1 parent 11d4efb commit ed9862a

2 files changed

Lines changed: 90 additions & 31 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ All notable changes to this project will be documented in this file.
3737

3838
- Calibrated `wp_ajax handlers without nonce validation` detection in `dist/bin/check-performance.sh` to catch missing CSRF protection reliably:
3939
- Replaced pipe-based `safe_file_iterator ... | while` loop with process substitution `while ...; done < <(...)` so failure flags and counters are preserved (no subshell scope loss)
40-
- Improved handler-to-nonce coverage logic by comparing unique `wp_ajax_*` registrations to nonce checks instead of only checking whether any nonce exists in the file
40+
- Upgraded from file-level to endpoint-level verification by mapping each `add_action('wp_ajax_*', callback)` registration to its callback and checking that callback body for `check_ajax_referer()`, `wp_verify_nonce()`, or `check_admin_referer()`
4141
- Fixed grep-count fallback handling (`grep -c ... || true`) to avoid malformed `0\n0` values during arithmetic comparisons
42-
- Verified against the Bloomz universal child theme case where 27 AJAX endpoints were missing nonce verification
42+
- Verified against the Bloomz universal child theme case (issue #768 scope), where the check now reports per-endpoint missing nonce findings instead of coarse file-level flags
4343

4444
- N+1 pattern findings now include the actual source code line in the report. Previously the `code` field was empty because `find_meta_in_loop_line` only returned the line number without extracting the source text
4545

dist/bin/check-performance.sh

Lines changed: 88 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2363,6 +2363,46 @@ find_callback_capability_check() {
23632363
return 1 # No capability check found
23642364
}
23652365

2366+
# Find callback function in same file and check for nonce verification
2367+
# Usage: find_callback_nonce_check "file.php" "callback_function_name"
2368+
# Returns: 0 if nonce verification found, 1 if not found
2369+
find_callback_nonce_check() {
2370+
local file="$1"
2371+
local callback_name="$2"
2372+
2373+
# Sanitize callback name (remove quotes/whitespace and normalize Class::method)
2374+
callback_name=$(echo "$callback_name" | sed "s/['\"]//g" | tr -d '[:space:]')
2375+
callback_name="${callback_name##*::}"
2376+
2377+
# Skip if callback is empty or looks dynamic
2378+
if [ -z "$callback_name" ] || [[ "$callback_name" =~ ^\$ ]] || [[ "$callback_name" =~ ^function ]]; then
2379+
return 1
2380+
fi
2381+
2382+
local func_line
2383+
func_line=$(grep -n "^[[:space:]]*function[[:space:]]\+${callback_name}[[:space:]]*(" "$file" 2>/dev/null | head -1 | cut -d: -f1)
2384+
2385+
if [ -z "$func_line" ]; then
2386+
func_line=$(grep -n "^[[:space:]]*\(public\|private\|protected\)\([[:space:]]\+static\)\?[[:space:]]\+function[[:space:]]\+${callback_name}[[:space:]]*(" "$file" 2>/dev/null | head -1 | cut -d: -f1)
2387+
fi
2388+
2389+
if [ -z "$func_line" ]; then
2390+
return 1
2391+
fi
2392+
2393+
local range function_start function_end func_body
2394+
range=$(get_function_scope_range "$file" "$func_line" 80)
2395+
function_start=${range%%:*}
2396+
function_end=${range##*:}
2397+
func_body=$(sed -n "${function_start},${function_end}p" "$file" 2>/dev/null || true)
2398+
2399+
if echo "$func_body" | grep -qE "check_ajax_referer[[:space:]]*\(|wp_verify_nonce[[:space:]]*\(|check_admin_referer[[:space:]]*\("; then
2400+
return 0
2401+
fi
2402+
2403+
return 1
2404+
}
2405+
23662406
# Conditional echo - only outputs in text mode
23672407
text_echo() {
23682408
if [ "$OUTPUT_FORMAT" = "text" ]; then
@@ -4438,43 +4478,62 @@ if [ -n "$AJAX_FILES" ]; then
44384478
# SAFEGUARD: Use safe_file_iterator() instead of "for file in $AJAX_FILES"
44394479
# File paths with spaces will break the loop without this helper (see common-helpers.sh)
44404480
# NOTE: Process substitution < <(...) is used instead of pipe | while to avoid subshell
4441-
# scoping pipe creates a subshell where AJAX_NONCE_FAIL and AJAX_NONCE_FINDING_COUNT
4481+
# scoping - pipe creates a subshell where AJAX_NONCE_FAIL and AJAX_NONCE_FINDING_COUNT
44424482
# changes are lost when the subshell exits, causing the check to always report "passed".
44434483
while IFS= read -r file; do
4444-
# Count add_action('wp_ajax_*') registrations (both authenticated and nopriv variants)
4445-
# NOTE: grep -c always outputs "0" on no match (exit 1); use || true not || echo 0
4446-
# to avoid capturing "0\n0" which breaks integer comparisons downstream.
4447-
handler_count=$(grep -cE "add_action[[:space:]]*\([[:space:]]*['\"]wp_ajax_" "$file" 2>/dev/null || true)
4448-
nonce_count=$(grep -cE "check_ajax_referer[[:space:]]*\(|wp_verify_nonce[[:space:]]*\(" "$file" 2>/dev/null || true)
4449-
4450-
if [ -z "$handler_count" ] || [ "$handler_count" -eq 0 ]; then
4484+
registrations=$(grep -nE "add_action[[:space:]]*\([[:space:]]*['\"]wp_ajax_" "$file" 2>/dev/null || true)
4485+
if [ -z "$registrations" ]; then
44514486
continue
44524487
fi
44534488

4454-
# Flag if zero nonce calls, OR if nonce calls are significantly fewer than handler
4455-
# registrations (each unique action appears twice: wp_ajax_ + wp_ajax_nopriv_,
4456-
# so nonce_count should be at least half the handler registrations).
4457-
# A file with 14 add_action lines and 0 nonce calls is clearly unprotected.
4458-
# A file with 14 add_action lines and 1-2 nonce calls is likely under-protected.
4459-
unique_handlers=$(grep -E "add_action[[:space:]]*\([[:space:]]*['\"]wp_ajax_" "$file" 2>/dev/null \
4460-
| grep -oE "'wp_ajax_[^']+'" | sed "s/wp_ajax_nopriv_/wp_ajax_/" | sort -u | wc -l | tr -d '[:space:]')
4461-
sufficient_nonces=$(( ${unique_handlers:-0} > 0 ? ${unique_handlers:-0} : 1 ))
4489+
# Build endpoint map: normalize wp_ajax_nopriv_* to wp_ajax_* and pair it with callback.
4490+
# This allows endpoint-level verification instead of file-level nonce counting.
4491+
endpoint_rows=""
4492+
while IFS= read -r match; do
4493+
[ -z "$match" ] && continue
4494+
lineno="${match%%:*}"
4495+
code="${match#*:}"
44624496

4463-
if [ "${nonce_count:-0}" -ge "$sufficient_nonces" ]; then
4464-
continue
4465-
fi
4497+
action=$(echo "$code" | sed -nE "s/.*add_action[[:space:]]*\([[:space:]]*['\"](wp_ajax_[^'\"]+)['\"][[:space:]]*,.*/\1/p")
4498+
callback_name=$(echo "$code" | sed -nE "s/.*add_action[[:space:]]*\([[:space:]]*['\"]wp_ajax_[^'\"]+['\"][[:space:]]*,[[:space:]]*['\"]([^'\"]+)['\"].*/\1/p")
44664499

4467-
if should_suppress_finding "wp-ajax-no-nonce" "$file"; then
4468-
continue
4469-
fi
4500+
if [ -z "$callback_name" ]; then
4501+
callback_name=$(echo "$code" | sed -nE "s/.*\[[^,]*,[[:space:]]*['\"]([^'\"]+)['\"].*/\1/p")
4502+
fi
4503+
if [ -z "$callback_name" ]; then
4504+
callback_name=$(echo "$code" | sed -nE "s/.*array[[:space:]]*\([^,]*,[[:space:]]*['\"]([^'\"]+)['\"].*/\1/p")
4505+
fi
4506+
4507+
[ -z "$action" ] && continue
4508+
normalized_action=$(echo "$action" | sed 's/^wp_ajax_nopriv_/wp_ajax_/')
4509+
endpoint_rows="${endpoint_rows}${normalized_action}\t${callback_name}\t${lineno}\t${code}\n"
4510+
done <<< "$registrations"
4511+
4512+
[ -z "$endpoint_rows" ] && continue
4513+
4514+
unique_endpoints=$(printf "%b" "$endpoint_rows" | awk -F '\t' '!seen[$1 FS $2]++')
4515+
4516+
while IFS= read -r endpoint; do
4517+
[ -z "$endpoint" ] && continue
4518+
4519+
normalized_action=$(echo "$endpoint" | cut -f1)
4520+
callback_name=$(echo "$endpoint" | cut -f2)
4521+
lineno=$(echo "$endpoint" | cut -f3)
4522+
code=$(echo "$endpoint" | cut -f4-)
4523+
4524+
if should_suppress_finding "wp-ajax-no-nonce" "$file"; then
4525+
continue
4526+
fi
4527+
4528+
if find_callback_nonce_check "$file" "$callback_name"; then
4529+
continue
4530+
fi
44704531

4471-
lineno=$(grep -n "add_action.*wp_ajax_" "$file" 2>/dev/null | head -1 | cut -d: -f1)
4472-
code=$(grep -n "add_action.*wp_ajax_" "$file" 2>/dev/null | head -1 | cut -d: -f2-)
4473-
missing=$(( ${unique_handlers:-0} - ${nonce_count:-0} < 0 ? 0 : ${unique_handlers:-0} - ${nonce_count:-0} ))
4474-
text_echo " $file: $unique_handlers wp_ajax handler(s), only $nonce_count nonce check(s) — $missing handler(s) likely missing CSRF protection"
4475-
add_json_finding "ajax-no-nonce" "error" "$AJAX_NONCE_SEVERITY" "$file" "${lineno:-0}" "wp_ajax handlers missing CSRF nonce verification ($unique_handlers handler(s), $nonce_count nonce check(s))" "$code"
4476-
AJAX_NONCE_FAIL=true
4477-
((AJAX_NONCE_FINDING_COUNT++))
4532+
text_echo " $file:$lineno ${normalized_action} callback '${callback_name}' missing nonce verification"
4533+
add_json_finding "ajax-no-nonce" "error" "$AJAX_NONCE_SEVERITY" "$file" "${lineno:-0}" "wp_ajax endpoint '${normalized_action}' callback '${callback_name}' missing CSRF nonce verification" "$code"
4534+
AJAX_NONCE_FAIL=true
4535+
((AJAX_NONCE_FINDING_COUNT++))
4536+
done <<< "$unique_endpoints"
44784537
done < <(safe_file_iterator "$AJAX_FILES")
44794538
fi
44804539
if [ "$AJAX_NONCE_FAIL" = true ]; then

0 commit comments

Comments
 (0)