Skip to content

fix: posts endpoint by allowlisting query args#4482

Merged
vytisbulkevicius merged 2 commits intodevelopmentfrom
bugfix/pro/3140
Mar 9, 2026
Merged

fix: posts endpoint by allowlisting query args#4482
vytisbulkevicius merged 2 commits 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

pirate-bot commented Mar 5, 2026

Plugin build for 9e42973 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.
@vytisbulkevicius vytisbulkevicius merged commit 0525a08 into development Mar 9, 2026
14 checks passed
@vytisbulkevicius vytisbulkevicius deleted the bugfix/pro/3140 branch March 9, 2026 11:18
@pirate-bot
Copy link
Collaborator

🎉 This PR is included in version 4.2.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Mar 9, 2026
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. released Indicate that an issue has been resolved and released in a particular version of the product.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants