diff --git a/.changeset/spotty-mice-behave.md b/.changeset/spotty-mice-behave.md new file mode 100644 index 00000000..4ce34cdf --- /dev/null +++ b/.changeset/spotty-mice-behave.md @@ -0,0 +1,10 @@ +--- +"@wpengine/wpgraphql-webhooks-wordpress-plugin": patch +--- + +fix: security improvements for webhooks plugin + +- Enhanced input validation and sanitization +- Improved output escaping +- Strengthened authorization checks +- Added additional security hardening measures diff --git a/plugins/wp-graphql-webhooks/src/Admin/WebhooksAdmin.php b/plugins/wp-graphql-webhooks/src/Admin/WebhooksAdmin.php index 3ad67163..a699bdaf 100644 --- a/plugins/wp-graphql-webhooks/src/Admin/WebhooksAdmin.php +++ b/plugins/wp-graphql-webhooks/src/Admin/WebhooksAdmin.php @@ -168,7 +168,7 @@ private function verify_admin_permission(): bool { * @return bool True if nonce is valid, false otherwise. */ private function verify_nonce( string $nonce_name, string $action ): bool { - if ( ! isset( $_REQUEST[ $nonce_name ] ) || ! wp_verify_nonce( $_REQUEST[ $nonce_name ], $action ) ) { + if ( ! isset( $_REQUEST[ $nonce_name ] ) || ! wp_verify_nonce( wp_unslash( $_REQUEST[ $nonce_name ] ), $action ) ) { wp_die( __( 'Security check failed.', 'wp-graphql-webhooks' ) ); return false; } @@ -185,11 +185,6 @@ public function handle_webhook_save() { wp_die( __( 'Unauthorized', 'wp-graphql-webhooks' ) ); } - $webhook_id = isset( $_POST['webhook_id'] ) ? intval( $_POST['webhook_id'] ) : 0; - if ( ! $this->verify_admin_permission() || ! $this->verify_nonce( 'webhook_nonce', 'webhook_save' ) ) { - wp_die( __( 'Unauthorized', 'wp-graphql-webhooks' ) ); - } - $webhook_id = isset( $_POST['webhook_id'] ) ? intval( $_POST['webhook_id'] ) : 0; $webhook = new Webhook( $webhook_id, @@ -375,7 +370,7 @@ public function ajax_test_webhook(): void { ] ); } - if ( ! current_user_can( 'manage_options' ) ) { + if ( ! $this->verify_admin_permission() ) { wp_send_json_error( [ 'message' => __( 'You do not have permission to test webhooks.', 'wp-graphql-webhooks' ), 'error_code' => 'insufficient_permissions' diff --git a/plugins/wp-graphql-webhooks/src/Admin/WebhooksListTable.php b/plugins/wp-graphql-webhooks/src/Admin/WebhooksListTable.php index 9ed03cdd..480da016 100644 --- a/plugins/wp-graphql-webhooks/src/Admin/WebhooksListTable.php +++ b/plugins/wp-graphql-webhooks/src/Admin/WebhooksListTable.php @@ -92,13 +92,13 @@ public function process_bulk_action() { } // Verify nonce - if ( ! isset( $_REQUEST['_wpnonce'] ) || ! wp_verify_nonce( $_REQUEST['_wpnonce'], 'bulk-' . $this->_args['plural'] ) ) { - wp_die( __( 'Security check failed.', 'wp-graphql-webhooks' ) ); + if ( ! isset( $_REQUEST['_wpnonce'] ) || ! wp_verify_nonce( wp_unslash( $_REQUEST['_wpnonce'] ), 'bulk-' . $this->_args['plural'] ) ) { + wp_die( esc_html__( 'Security check failed.', 'wp-graphql-webhooks' ) ); } // Check permissions if ( ! current_user_can( 'manage_options' ) ) { - wp_die( __( 'You do not have sufficient permissions to access this page.', 'wp-graphql-webhooks' ) ); + wp_die( esc_html__( 'You do not have sufficient permissions to access this page.', 'wp-graphql-webhooks' ) ); } // Get selected webhooks @@ -137,8 +137,8 @@ public function prepare_items() { $total_items = count( $webhooks ); // Handle sorting - $orderby = ! empty( $_GET['orderby'] ) ? $_GET['orderby'] : 'name'; - $order = ! empty( $_GET['order'] ) ? $_GET['order'] : 'asc'; + $orderby = ! empty( $_GET['orderby'] ) ? sanitize_key( $_GET['orderby'] ) : 'name'; + $order = ! empty( $_GET['order'] ) ? sanitize_key( $_GET['order'] ) : 'asc'; usort( $webhooks, function( $a, $b ) use ( $orderby, $order ) { $result = 0; @@ -253,7 +253,7 @@ public function column_name( $item ) { * Display when no items */ public function no_items() { - _e( 'No webhooks found.', 'wp-graphql-webhooks' ); + esc_html_e( 'No webhooks found.', 'wp-graphql-webhooks' ); } /** diff --git a/plugins/wp-graphql-webhooks/src/Events/SmartCacheWebhookManager.php b/plugins/wp-graphql-webhooks/src/Events/SmartCacheWebhookManager.php index 7db05313..20f5f37a 100644 --- a/plugins/wp-graphql-webhooks/src/Events/SmartCacheWebhookManager.php +++ b/plugins/wp-graphql-webhooks/src/Events/SmartCacheWebhookManager.php @@ -116,7 +116,9 @@ private function trigger_webhooks( string $event, array $payload ): void { $payload['uri'] = $payload['path'] ?? ''; } - error_log( "[Webhook] Triggering webhooks for event: $event with payload: " . var_export( $payload, true ) ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "[Webhook] Triggering webhooks for event: $event with payload: " . var_export( $payload, true ) ); + } do_action( 'graphql_webhooks_before_trigger', $event, $payload ); @@ -240,7 +242,7 @@ public function get_path_from_key( $key ) { } if ( ! empty( $permalink ) && is_string( $permalink ) && ! is_wp_error( $permalink ) ) { - $parsed_path = parse_url( $permalink, PHP_URL_PATH ); + $parsed_path = wp_parse_url( $permalink, PHP_URL_PATH ); if ( $parsed_path !== false ) { $path = $parsed_path; error_log( "[Webhook] Final path for key $key: $path" ); diff --git a/plugins/wp-graphql-webhooks/src/Handlers/WebhookHandler.php b/plugins/wp-graphql-webhooks/src/Handlers/WebhookHandler.php index eecd7d12..1e4c4278 100644 --- a/plugins/wp-graphql-webhooks/src/Handlers/WebhookHandler.php +++ b/plugins/wp-graphql-webhooks/src/Handlers/WebhookHandler.php @@ -22,12 +22,14 @@ class WebhookHandler implements Handler { public function handle( Webhook $webhook, array $payload ): void { // Log webhook dispatch initiation $dispatch_timestamp = current_time( 'mysql' ); - error_log( "\n========== WEBHOOK DISPATCH ==========" ); - error_log( "Timestamp: {$dispatch_timestamp}" ); - error_log( "Webhook: {$webhook->name} (ID: {$webhook->id})" ); - error_log( "Event: {$webhook->event}" ); - error_log( "Target URL: {$webhook->url}" ); - error_log( "Method: {$webhook->method}" ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "\n========== WEBHOOK DISPATCH ==========" ); + error_log( "Timestamp: {$dispatch_timestamp}" ); + error_log( "Webhook: {$webhook->name} (ID: {$webhook->id})" ); + error_log( "Event: {$webhook->event}" ); + error_log( "Target URL: {$webhook->url}" ); + error_log( "Method: {$webhook->method}" ); + } $args = [ 'headers' => $webhook->headers ?: [ 'Content-Type' => 'application/json' ], @@ -52,7 +54,9 @@ public function handle( Webhook $webhook, array $payload ): void { if ( strtoupper( $webhook->method ) === 'GET' ) { $url = add_query_arg( $payload, $webhook->url ); $args['method'] = 'GET'; - error_log( "Payload (GET query params): " . wp_json_encode( $payload ) ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "Payload (GET query params): " . wp_json_encode( $payload ) ); + } } else { $url = $webhook->url; $args['method'] = strtoupper( $webhook->method ); @@ -63,20 +67,28 @@ public function handle( Webhook $webhook, array $payload ): void { $args['headers']['Content-Type'] = 'application/json'; } - error_log( "Payload ({$args['method']} body): " . $args['body'] ); - error_log( "Payload size: " . strlen( $args['body'] ) . " bytes" ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "Payload ({$args['method']} body): " . $args['body'] ); + error_log( "Payload size: " . strlen( $args['body'] ) . " bytes" ); + } } // Log headers - error_log( "Headers: " . wp_json_encode( $args['headers'] ) ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "Headers: " . wp_json_encode( $args['headers'] ) ); + } // For test mode or debugging, optionally use blocking mode if ( apply_filters( 'graphql_webhooks_test_mode', false, $webhook ) ) { $args['blocking'] = true; - error_log( "Test mode enabled - using blocking request" ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "Test mode enabled - using blocking request" ); + } } - error_log( "====================================\n" ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "====================================\n" ); + } // Send the webhook $start_time = microtime( true ); @@ -85,7 +97,7 @@ public function handle( Webhook $webhook, array $payload ): void { $duration = round( ( $end_time - $start_time ) * 1000, 2 ); // Log response if in blocking mode - if ( $args['blocking'] ) { + if ( $args['blocking'] && defined( 'WP_DEBUG' ) && WP_DEBUG ) { if ( is_wp_error( $response ) ) { error_log( "\n========== WEBHOOK ERROR ==========" ); error_log( "❌ ERROR: " . $response->get_error_message() ); diff --git a/plugins/wp-graphql-webhooks/src/Rest/WebhookTestEndpoint.php b/plugins/wp-graphql-webhooks/src/Rest/WebhookTestEndpoint.php index 5c07bd31..7f22db34 100644 --- a/plugins/wp-graphql-webhooks/src/Rest/WebhookTestEndpoint.php +++ b/plugins/wp-graphql-webhooks/src/Rest/WebhookTestEndpoint.php @@ -86,15 +86,16 @@ public function test_webhook( WP_REST_Request $request ): WP_REST_Response|WP_Er // Log test initiation $test_timestamp = current_time( 'mysql' ); - error_log( "\n========== WEBHOOK TEST INITIATED ==========" ); - error_log( "Timestamp: {$test_timestamp}" ); - error_log( "Webhook ID: {$webhook_id}" ); - error_log( "Webhook Name: {$webhook->name}" ); - error_log( "Target URL: {$webhook->url}" ); - error_log( "HTTP Method: {$webhook->method}" ); - error_log( "Event: {$webhook->event}" ); - error_log( "Headers: " . wp_json_encode( $webhook->headers ) ); - error_log( "==========================================\n" ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "\n========== WEBHOOK TEST INITIATED ==========" ); + error_log( "Timestamp: {$test_timestamp}" ); + error_log( "Webhook ID: {$webhook_id}" ); + error_log( "Webhook Name: {$webhook->name}" ); + // Do not log sensitive URL and headers + error_log( "HTTP Method: {$webhook->method}" ); + error_log( "Event: {$webhook->event}" ); + error_log( "==========================================\n" ); + } // Create test payload $test_payload = [ @@ -107,7 +108,7 @@ public function test_webhook( WP_REST_Request $request ): WP_REST_Response|WP_Er ], 'test_data' => [ 'message' => 'This is a test webhook payload', - 'triggered_by' => wp_get_current_user()->user_login, + 'triggered_by' => 'admin', 'site_url' => get_site_url(), ], ]; @@ -124,11 +125,13 @@ public function test_webhook( WP_REST_Request $request ): WP_REST_Response|WP_Er $end_time = microtime( true ); $duration = round( ( $end_time - $start_time ) * 1000, 2 ); // Convert to milliseconds - error_log( "\n========== WEBHOOK TEST COMPLETED ==========" ); - error_log( "✅ SUCCESS: Test webhook dispatched" ); - error_log( "Duration: {$duration}ms" ); - error_log( "Completed at: " . current_time( 'mysql' ) ); - error_log( "==========================================\n" ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "\n========== WEBHOOK TEST COMPLETED ==========" ); + error_log( "✅ SUCCESS: Test webhook dispatched" ); + error_log( "Duration: {$duration}ms" ); + error_log( "Completed at: " . current_time( 'mysql' ) ); + error_log( "==========================================\n" ); + } return new WP_REST_Response( [ @@ -147,10 +150,12 @@ public function test_webhook( WP_REST_Request $request ): WP_REST_Response|WP_Er 200 ); } catch ( \Exception $e ) { - error_log( "\n========== WEBHOOK TEST ERROR ==========" ); - error_log( "❌ ERROR: " . $e->getMessage() ); - error_log( "Stack trace: " . $e->getTraceAsString() ); - error_log( "========================================\n" ); + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( "\n========== WEBHOOK TEST ERROR ==========" ); + error_log( "❌ ERROR: " . $e->getMessage() ); + // Do not log stack trace as it may contain sensitive information + error_log( "========================================\n" ); + } return new WP_Error( 'webhook_test_failed', diff --git a/plugins/wp-graphql-webhooks/src/Services/PluginServiceLocator.php b/plugins/wp-graphql-webhooks/src/Services/PluginServiceLocator.php index 6fbd9d76..03def7ef 100644 --- a/plugins/wp-graphql-webhooks/src/Services/PluginServiceLocator.php +++ b/plugins/wp-graphql-webhooks/src/Services/PluginServiceLocator.php @@ -26,7 +26,7 @@ public function has(string $name): bool { public function get(string $name) { if (!isset($this->factories[$name])) { - throw new UnexpectedValueException("Service not found: {$name}"); + throw new UnexpectedValueException( esc_html( "Service not found: {$name}" ) ); } if (!isset($this->instances[$name])) {