Skip to:
Content

bbPress.org

Opened 7 years ago

Closed 7 years ago

#2367 closed defect (fixed)

Functions that output URL's do not use esc_url()

Reported by: johnjamesjacoby Owned by: johnjamesjacoby
Milestone: 2.4 Priority: high
Severity: normal Version: 2.0
Component: Tools - Code Improvements Keywords: needs-patch 2nd-opinion
Cc: nashwan.doaqan@…

Description

The following functions that output URL's are not automatically ran through esc_url():

  • bbp_ajax_url()
  • bbp_forums_url()
  • bbp_topics_url()
  • bbp_view_url()
  • bbp_forum_permalink()
  • bbp_forum_last_reply_url()
  • bbp_forum_last_topic_permalink()
  • bbp_forum_last_reply_permalink()
  • bbp_reply_permalink()
  • bbp_reply_url()
  • bbp_reply_edit_url()
  • bbp_reply_author_url()
  • bbp_search_url()
  • bbp_search_results_url()
  • bbp_topic_permalink()
  • bbp_topic_author_url()
  • bbp_topic_last_reply_permalink()
  • bbp_topic_last_reply_url()
  • bbp_topic_edit_url()
  • bbp_user_profile_url()
  • bbp_user_profile_edit_url()
  • bbp_user_topics_created_url()
  • bbp_user_replies_created_url()
  • bbp_favorites_permalink()
  • bbp_subscriptions_permalink()

At first this sounds really bad, but it's actually not. All of these functions either use WordPress core functions like get_permalink(), or have _get_ equivalents that already safely assemble the entire URL (see: bbp_get_user_profile_url())

Opening this ticket here to identify functions that might not need esc_url() treatment; I have a hunch they'll all benefit from esc_url() though.

Originally identified by Gemma Moore via a related issue to the security@ email list.

Attachments (1)

2367.patch (10.4 KB) - added by johnjamesjacoby 7 years ago.

Download all attachments as: .zip

Change History (5)

#1 @alex-ye
7 years ago

  • Cc nashwan.doaqan@… added

Very Nice :)

#2 @johnjamesjacoby
7 years ago

In 5037:

For all template functions that output URL's, always echo an escaped value using esc_url(). See #2367.

#3 @johnjamesjacoby
7 years ago

In 5040:

More esc_url() improvements, and practice late-escaping where we were otherwise passing around escaped URL variables. See #2367.

#4 @johnjamesjacoby
7 years ago

  • Resolution set to fixed
  • Status changed from new to closed

There will be ongoing improvements here, but the above changesets take care of the large majority of them, so marking as fixed.

Note: See TracTickets for help on using tickets.