Skip to:
Content

BuddyPress.org

Opened 10 years ago

Last modified 4 years ago

#2776 new defect (bug)

Most content is double-escaped in the database

Reported by: DJPaul Owned by:
Milestone: Awaiting Contributions Priority: major
Severity: normal Version:
Component: Core Keywords:
Cc: nacin

Description (last modified by DJPaul)

Throughout BuddyPress, a lot of input (i.e. xprofile data, group name, group description) is being stored double-escaped in the database. This is demonstrated by creating a group with an apostrophe in its group description field, and then by creating a regular WP post with the same phrase, and comparing the contents of the database tables.

This is because WordPress, in wp_magic_quotes(), escapes everything in $_POST, $_GET and $_COOKIE. BuddyPress needs to stripslashes() on relevant content before we put it into the database, as $wpdb->prepare() escapes the input again.
This problem hasn't been very visible due to stripslashes() being added to most template tag's output functions, and a few local workarounds, but ticket #1209 led me to find this issue.

Related:
#1209
#2283

Attachments (4)

bpcore.patch (2.3 KB) - added by DJPaul 10 years ago.
bp_core_action_search_site
bpcore-2.patch (6.6 KB) - added by DJPaul 10 years ago.
bp_core_action_search_site 2
2776-xprofile-1.patch (13.2 KB) - added by DJPaul 10 years ago.
xprofile 1
2776-xprofile-2.patch (13.1 KB) - added by DJPaul 10 years ago.
xprofile 2

Download all attachments as: .zip

Change History (17)

@DJPaul
10 years ago

bp_core_action_search_site

#1 @DJPaul
10 years ago

The patch for bp_core_action_search_site fixes this issue when performing a search from the site-wide search box in BP-Default's header, as well as improving logic of the patch and catching a few WP_DEBUG warnings.

@DJPaul
10 years ago

bp_core_action_search_site 2

#2 @DJPaul
10 years ago

bpcore-2.patch is updated to include a bit I missed including in the patch, and also handles the other directories' search boxes in the same way.

#3 @boonebgorges
10 years ago

bpcore-2.patch looks clean to me.

#4 @djpaul
10 years ago

(In [3511]) Fix double-escaping of search string when performing search from the site-wide search box. Partially addresses #2776.

@DJPaul
10 years ago

xprofile 1

#5 @DJPaul
10 years ago

  • Description modified (diff)

T

#6 @DJPaul
10 years ago

xprofile-1 address double-escaping of data in the database throughout the xprofile component.
This patch also has fixes for #2694 and #2777 as they are related. It also reverts [3509] #2283 as the issue is best addressed when data is put into the database, rather than on output.

I have tested it a lot and it seems to work as expected, both in the database as well as in the front-end and admin UIs. I have not tested the impact of the patch on existing xprofile data; search will not work on previously-escaped characters, such as apostrophes, but we need to check if we get any slashes output on the front-end and admin UIs. If so, we may need to (re)add extra stripslashes() to maintain this backwards compatibility.

#7 @nacin
10 years ago

  • Cc nacin added

@DJPaul
10 years ago

xprofile 2

#8 @DJPaul
10 years ago

xprofile-2 patch removes a filter which is unnecessary.

#9 @boonebgorges
10 years ago

DJPaul, am I correct that you will be punting this?

#10 @DJPaul
10 years ago

  • Milestone changed from 1.3 to 1.4

Yes, not enough time to go through this properly.

#11 @DJPaul
9 years ago

  • Milestone changed from 1.6 to Future Release
  • Severity set to normal

#12 @boonebgorges
7 years ago

In 8156:

Run stripslashes filter on activity strings earlier than priority 10

Running stripslashes() on priority 10 caused race conditions with other filters
on the same content. This, in turn, causes conflicts with wptexturize(), which
cannot properly parse certain character combinations due to the incorrect
presence of escaping slashes. This problem exhibited itself most obviously
with the use of guillemet-style quotation marks (the slashes fooled
wptexturize() into thinking that the opening quote was actually the closing
one), but could also occur with other formatting rules.

Because the requirement to stripslashes() is due to BuddyPress's incorrect
escaping of much input content, we work around the race condition by running
stripslashes_deep() earlier than priority 10. This ensures that plugins hooking
to these filters with the default priority can expect properly formatted and
sanitized content. See #2776.

Props chouf1, imath, needle

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


4 years ago

Note: See TracTickets for help on using tickets.