Skip to content

[Security] Add nonce verification to all state-changing operations #38

@pbking

Description

@pbking

Issue: Missing Nonce Verification in State-Changing Operations

Description

While the main REST API endpoints have nonce verification in their permission callbacks, several filter hooks that perform state-changing operations lack explicit nonce verification. This could potentially allow bypass of security checks in edge cases.

Current State

✅ Properly Protected

  • write_permission_callback() - Has nonce verification (line 93-98)
  • process_theme_patterns() - Protected by write_permission_callback
  • handle_hijack_block_update() - Has nonce verification (line 433-438)

❌ Missing Nonce Verification

  1. handle_hijack_block_delete() (line 330)

    • Performs: Deletes posts and files
    • Issue: No nonce verification before wp_delete_post()
  2. handle_block_to_pattern_conversion() (line 582)

    • Performs: Modifies request body content
    • Issue: No nonce verification for PUT/POST operations

Security Risk

Medium - While these hooks are called within the REST API context which has its own authentication, adding explicit nonce verification would provide defense-in-depth security.

Affected Files

  • /includes/class-pattern-builder-api.php

Recommended Solution

Add nonce verification to all state-changing filter hooks:

// Example for handle_hijack_block_delete
function handle_hijack_block_delete( $response, $server, $request ) {
    // Add nonce verification
    $nonce = $request->get_header( 'X-WP-Nonce' );
    if ( ! $nonce || ! wp_verify_nonce( $nonce, 'wp_rest' ) ) {
        return new WP_Error(
            'rest_cookie_invalid_nonce',
            __( 'Cookie nonce is invalid', 'pattern-builder' ),
            array( 'status' => 403 )
        );
    }
    
    // Existing code...
}

Implementation Checklist

  • Add nonce verification to handle_hijack_block_delete()
  • Add nonce verification to handle_block_to_pattern_conversion()
  • Verify all state-changing operations have nonce checks
  • Add unit tests for nonce verification
  • Document security requirements in code comments

Benefits

  • Defense in Depth: Multiple layers of security checks
  • Consistency: All state-changing operations follow same security pattern
  • WordPress Standards: Aligns with WordPress security best practices
  • CSRF Protection: Prevents cross-site request forgery attacks

Testing Requirements

  • Verify operations fail without valid nonce
  • Verify operations succeed with valid nonce
  • Test doesn't break existing functionality
  • Confirm no performance impact

Priority: Medium - Adds important security layer to prevent potential bypasses

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions