Skip to content

fix: posts endpoint by allowlisting query args#4482

Open
girishpanchal30 wants to merge 1 commit intodevelopmentfrom
bugfix/pro/3140
Open

fix: posts endpoint by allowlisting query args#4482
girishpanchal30 wants to merge 1 commit intodevelopmentfrom
bugfix/pro/3140

Conversation

@girishpanchal30
Copy link
Contributor

Summary

Harden public nv/v1/posts endpoint by allow listing query args.

Check before Pull Request is ready:

Closes https://github.com/Codeinwp/neve-pro-addon/issues/3140.

@pirate-bot pirate-bot added the pr-checklist-incomplete The Pull Request checklist is incomplete. (automatic label) label Mar 5, 2026
@girishpanchal30 girishpanchal30 added the pr-checklist-skip Allow this Pull Request to skip checklist. label Mar 5, 2026
@pirate-bot pirate-bot added pr-checklist-complete The Pull Request checklist is complete. (automatic label) and removed pr-checklist-incomplete The Pull Request checklist is incomplete. (automatic label) labels Mar 5, 2026
@pirate-bot
Copy link
Collaborator

Plugin build for 721d2ad is ready 🛎️!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_number requests 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 lang parameter 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';
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$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.

Suggested change
$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';
}

Copilot uses AI. Check for mistakes.
$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' );
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
$safe_orderby = array( 'date', 'title', 'author', 'modified', 'comment_count', 'rand' );
$safe_orderby = array( 'date', 'title', 'author', 'modified', 'comment_count' );

Copilot uses AI. Check for mistakes.
Comment on lines +59 to +61
if ( $page_number > 100 ) {
return new \WP_REST_Response( '' );
}
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
if ( $page_number > 100 ) {
return new \WP_REST_Response( '' );
}

Copilot uses AI. Check for mistakes.
Comment on lines +369 to +377
$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';
}
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-checklist-complete The Pull Request checklist is complete. (automatic label) pr-checklist-skip Allow this Pull Request to skip checklist.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants