fix: sanitize GET params and add null checks in REST API association endpoints#1317
Open
thisismyurl wants to merge 1 commit into
Open
fix: sanitize GET params and add null checks in REST API association endpoints#1317thisismyurl wants to merge 1 commit into
thisismyurl wants to merge 1 commit into
Conversation
…endpoints - Sanitize container_id and field_id parameters using sanitize_text_field() - Add isset() checks before accessing $_GET values - Add null-check guards: return empty array if required params are missing - Sanitize options parameter to prevent injection through explode() - Matches existing sanitization pattern in get_attachment_data() Fixes htmlburger#1314: Unsanitized $_GET params and missing null checks in REST API Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Hardens REST router endpoints by sanitizing query parameters and avoiding undefined index notices when required params are missing.
Changes:
- Sanitize
container_id,field_id, andoptionsinputs read from$_GET - Add early returns when required parameters are missing (returning empty arrays)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+324
to
+326
| $container_id = isset( $_GET['container_id'] ) ? sanitize_text_field( $_GET['container_id'] ) : ''; | ||
| $field_id = isset( $_GET['field_id'] ) ? sanitize_text_field( $_GET['field_id'] ) : ''; | ||
| $options = isset( $_GET['options'] ) ? explode( ';', sanitize_text_field( $_GET['options'] ) ) : array(); |
Comment on lines
+329
to
+331
| if ( ! $container_id || ! $field_id ) { | ||
| return $return_value; | ||
| } |
Comment on lines
+377
to
+379
| if ( ! $container_id || ! $field_id ) { | ||
| return array(); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds missing input sanitization and null checks to REST API association endpoints, preventing unsanitized user input from reaching database operations.
Problem
The REST API endpoints for
get_association_data()andget_association_options()incore/REST_API/Router.phpwere directly accessing$_GET['container_id'],$_GET['field_id'], and$_GET['options']without sanitization or null checks. While the rest of the codebase properly sanitizes these parameters (see lines 367–368), these two methods were overlooked.This creates a potential security issue where unsanitized user input could reach database queries if not sanitized at the query layer.
Solution
Added proper input validation to both methods:
Same pattern applied to
get_association_options()and theoptionsparameter before it's used inexplode(). This matches the existing sanitization pattern already in use elsewhere in the same file.Changes
isset()checks to prevent undefined index noticessanitize_text_field()to string inputsAll changes are backwards compatible and defensive in nature.