Skip to:
Content

Opened 4 years ago

Closed 4 years ago

Last modified 12 months ago

#2826 closed enhancement (fixed)

Converting queries to use prepared statements

Reported by: glynwintle Owned by: johnjamesjacoby
Milestone: 2.6 Priority: normal
Severity: minor Version: trunk
Component: Component - Topics Keywords: needs-refresh
Cc: tom@…

Description

bbPress hardening by converting two database queries to use prepared sql queries. This is a case of reducing the attack surface, not saying that these are exploitable. There is no harm from doing this and possibly a gain.

The two functions are bbp_update_forum_topic_count_hidden and bbp_update_topic_reply_count_hidden.

I have attached a patch.

Attachments (1)

0002-Hardening-2.patch (2.1 KB) - added by glynwintle 4 years ago.
Patch

Download all attachments as: .zip

Change History (7)

@glynwintle
4 years ago

Patch

#1 @johnjamesjacoby
4 years ago

  • Component changed from General to Component - Topics
  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to Under Consideration
  • Severity changed from normal to minor
  • Type changed from defect to enhancement

Hi glynmintle,

Thanks for the patch. Unfortunately the status ID's are strings, so the attached patch will break these queries entirely.

I agree that the concatenation here is not very pretty, and since it isn't exploitable at this level, we can shift some priorities around until we get the patch right.

It looks like each needle in the IN haystack does not need to be wrapped in single quotes, so we might be able to eliminate the concatenation completely in the $post_status assignments.

#2 @netweb
4 years ago

I think we should also be including bbp_get_pending_status_id() for the 2.6 branch here.

#3 @netweb
4 years ago

  • Milestone changed from Under Consideration to 2.5.8

#4 @johnjamesjacoby
4 years ago

  • Keywords needs-refresh added; needs-patch removed
  • Milestone changed from 2.5.8 to 2.6

Moving to 2.6, since this isn't exploitable.

Let's get rid of the concatenation and look at adding bbp_get_pending_status_id() which makes sense to me at a glance.

#5 @johnjamesjacoby
4 years ago

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

In 5858:

Queries: Clean up post status concatenation in 2 queries.

This changeset moves array of "hidden" type post-statuses into their own variable to better match existing usages.

Fixes #2826.

#6 @tomdxw
12 months ago

  • Cc tom@… added
Note: See TracTickets for help on using tickets.