Skip to:
Content

Opened 8 years ago

Last modified 2 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 8 years ago.
bp_core_action_search_site
bpcore-2.patch (6.6 KB) - added by DJPaul 8 years ago.
bp_core_action_search_site 2
2776-xprofile-1.patch (13.2 KB) - added by DJPaul 8 years ago.
xprofile 1
2776-xprofile-2.patch (13.1 KB) - added by DJPaul 8 years ago.
xprofile 2

Download all attachments as: .zip

Change History (17)

@DJPaul
8 years ago

bp_core_action_search_site

#1 @DJPaul
8 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
8 years ago

bp_core_action_search_site 2

#2 @DJPaul
8 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
8 years ago

bpcore-2.patch looks clean to me.

#4 @djpaul
8 years ago

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

@DJPaul
8 years ago

xprofile 1

#5 @DJPaul
8 years ago

  • Description modified (diff)

T

#6 @DJPaul
8 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
8 years ago

  • Cc nacin added

@DJPaul
8 years ago

xprofile 2

#8 @DJPaul
8 years ago

xprofile-2 patch removes a filter which is unnecessary.

#9 @boonebgorges
8 years ago

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

#10 @DJPaul
8 years ago

  • Milestone changed from 1.3 to 1.4

Yes, not enough time to go through this properly.

#11 @DJPaul
7 years ago

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

#12 @boonebgorges
5 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.


2 years ago

Note: See TracTickets for help on using tickets.