IRL - Missed Detection: Unsanitized $_POST in AJAX handler

[mrtwebdesign]

Summary

WPCC reported "Unsanitized superglobal read ($_GET/$_POST)" as PASSED when the codebase contains raw $_POST usage without sanitization.

Scanned: Universal Child Theme 2024 (~12K LOC)
WPCC Version: 2.0.14

---

🔴 Missed Detection

Location: functions.php line 660

function fetch_search_data() {

    $response = new stdClass();
    $keyword = $_POST['keyword'];  // ❌ No isset(), no sanitization

    $prod_args = array(
        'post_type' => 'product',
        's' => $keyword,  // ← Raw user input goes into WP_Query
        'posts_per_page' => 3,
    );

    $prod_search = new WP_Query($prod_args);
    // ...
}

What WPCC reported:

{"name": "Unsanitized superglobal read ($_GET/$_POST)", "status": "passed", "findings_count": 0}

---

Additional Instance

Location: functions.php line 930

$selected_value = isset( $_GET['wholesale_sales_rep'] ) ? $_GET['wholesale_sales_rep'] : '';
echo '<option value="' . $value . '" ' . selected( $selected_value, $value, false ) . '>';

Here isset() is present but no sanitization before passing to selected(). While selected() does comparison only (lower risk), this is still unsanitized superglobal usage.

---

Why This Matters

1. Line 660 is in a public AJAX handler (wp_ajax_nopriv_fetch_search_data)
2. No isset() check - will throw PHP notice if $_POST['keyword'] not set
3. Raw input into WP_Query - while WP_Query's s parameter is relatively safe, this pattern is dangerous

---

Possible Detection Pattern Issue

The current pattern may require specific contexts (direct echo, database query) and miss:

• Simple variable assignment: $var = $_POST['key'];
• Assignment followed by use in function arguments

Current check might look for:

echo $_POST['x'];           // Caught
$wpdb->query($_GET['x']);   // Caught

But miss:

$keyword = $_POST['keyword'];  // Not caught
do_something($keyword);        // Dangerous use later

---

Suggested Detection Approach

Flag any $_GET/$_POST/$_REQUEST that:

1. Is assigned to a variable without sanitize_*, esc_*, absint(), intval(), etc. on the same line
2. OR is used directly in function arguments without sanitization wrapper

Suggested pattern:

# Match: $var = $_POST['key'] without sanitize wrapper
\$\w+\s*=\s*\$_(GET|POST|REQUEST)\s*\[(?!.*sanitize_|.*esc_|.*absint|.*intval)

---

Suggested Test Fixture

// fixtures/unsanitized-superglobal-assignment.php
// Expected: FAIL - Unsanitized superglobal

function process_form() {
    $name = $_POST['name'];           // ❌ Should flag
    $id = $_GET['id'];                // ❌ Should flag
    $safe = sanitize_text_field($_POST['email']);  // ✅ OK
    
    return $name . $id;
}

[noelsaw1]

Fixed in: #109

✅ Task Complete: Detection Gap Fixed
Summary
Successfully identified and fixed a critical detection gap in the scanner that affected all patterns using cached_grep when scanning files with spaces in their paths.

What Was Fixed
Version: 2.2.2 → 2.2.3

Issue: unsanitized-superglobal-read pattern (and 44+ other patterns) failed to detect violations in files with spaces in paths

Root Cause: xargs was splitting file paths on whitespace instead of newlines

Fix: Changed cached_grep to use null-delimited input with xargs -0

Changes Made
dist/bin/check-performance.sh (line 3354):
Changed: cat "$PHP_FILE_LIST" | xargs grep
To: tr '\n' '\0' < "$PHP_FILE_LIST" | xargs -0 grep
CHANGELOG.md:
Added v2.2.3 section with detailed root cause analysis
Documented the fix and lessons learned
Listed affected patterns and impact
Verification
✅ Unit test confirmed the fix works:

Old method: 0 findings (broken)
New method: Lines 660 and 930 detected correctly

Lessons Learned (from CHANGELOG)
Always test with file paths containing spaces (common in Local by Flywheel, MAMP)
xargs default behavior is unsafe for file paths; always use -0 with null-delimited input
Silent failures in grep pipelines can mask critical bugs

All tasks are now complete! The detection gap has been fixed, tested, and documented.