fix: posts endpoint by allowlisting query args#4482
fix: posts endpoint by allowlisting query args#4482girishpanchal30 wants to merge 1 commit intodevelopmentfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the public nv/v1/posts REST endpoint used by infinite scroll by restricting/sanitizing client-provided query arguments to reduce query manipulation and potential abuse.
Changes:
- Adds a hard cap for
page_numberrequests and ensures it’s treated as an integer. - Introduces a strict allowlist-based sanitization method for WP_Query args coming from the request body.
- Sanitizes the
langparameter before using it with Polylang/WPML behaviors.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $sanitized['s'] = sanitize_text_field( $sanitized['s'] ); | ||
| } | ||
| if ( isset( $sanitized['order'] ) ) { | ||
| $sanitized['order'] = in_array( strtoupper( $sanitized['order'] ), array( 'ASC', 'DESC' ), true ) ? $sanitized['order'] : 'DESC'; |
There was a problem hiding this comment.
$sanitized['order'] is passed through strtoupper() without ensuring it’s a string. A crafted JSON body can send order as an array/object, which can trigger a PHP 8 TypeError and 500 the endpoint. Add an is_string() (or is_scalar() + cast) guard before strtoupper(), and consider normalizing the stored value to ASC/DESC after validation.
| $sanitized['order'] = in_array( strtoupper( $sanitized['order'] ), array( 'ASC', 'DESC' ), true ) ? $sanitized['order'] : 'DESC'; | |
| if ( is_scalar( $sanitized['order'] ) ) { | |
| $order = strtoupper( (string) $sanitized['order'] ); | |
| $sanitized['order'] = in_array( $order, array( 'ASC', 'DESC' ), true ) ? $order : 'DESC'; | |
| } else { | |
| $sanitized['order'] = 'DESC'; | |
| } |
| $sanitized['order'] = in_array( strtoupper( $sanitized['order'] ), array( 'ASC', 'DESC' ), true ) ? $sanitized['order'] : 'DESC'; | ||
| } | ||
| if ( isset( $sanitized['orderby'] ) ) { | ||
| $safe_orderby = array( 'date', 'title', 'author', 'modified', 'comment_count', 'rand' ); |
There was a problem hiding this comment.
The allowlist for orderby includes rand, which in WP_Query generally results in expensive queries and can be abused for DoS on a public endpoint (contradicting the hardening goal noted in the docblock). Consider removing rand from the allowed list, or only allowing it for authenticated/privileged requests.
| $safe_orderby = array( 'date', 'title', 'author', 'modified', 'comment_count', 'rand' ); | |
| $safe_orderby = array( 'date', 'title', 'author', 'modified', 'comment_count' ); |
| if ( $page_number > 100 ) { | ||
| return new \WP_REST_Response( '' ); | ||
| } |
There was a problem hiding this comment.
Capping page_number at 100 can desync with the client-side maxPages value (localized from $wp_query->max_num_pages). On sites with >100 pages, infinite scroll will keep requesting pages 101..maxPages and receive empty responses, causing unnecessary repeated requests. Consider also capping the localized maxPages to 100 (or returning a signal the JS can use to stop requesting).
| if ( $page_number > 100 ) { | |
| return new \WP_REST_Response( '' ); | |
| } |
| $post_type = ! empty( $args['post_type'] ) ? $args['post_type'] : 'post'; | ||
| $post_type_obj = get_post_type_object( $post_type ); | ||
|
|
||
| // Only allow if post type exists and is publicly queryable. | ||
| if ( $post_type_obj && $post_type_obj->publicly_queryable ) { | ||
| $sanitized['post_type'] = $post_type; | ||
| } else { | ||
| $sanitized['post_type'] = 'post'; | ||
| } |
There was a problem hiding this comment.
post_type is taken directly from the decoded JSON body and passed to get_post_type_object()/returned in $sanitized without type normalization. If an attacker supplies a non-string (e.g., array), this can raise warnings (array-to-string conversion) and makes the sanitization less robust. Ensure post_type is a string (e.g., is_string + sanitize_key) before using it, otherwise fall back to 'post'.
Summary
Harden public
nv/v1/postsendpoint by allow listing query args.Check before Pull Request is ready:
Closes https://github.com/Codeinwp/neve-pro-addon/issues/3140.