_ck_
Moderator
If you are using the Private Messaging plugin
by Joshua Hutchins / ardentfrost.rayd.org
or even Detective's mod version of it, you need to take it offline immediately.
There are some very serious, multiple security problems with it.
If you are running it, your site can easily be hacked.
Please take this warning seriously, it's not worth the headache.
Make sure you delete not only the plugin file
but the additional files it uses in the bbPress root.
_ck_
Moderator
This is a quick and dirty security fix. Only tested on a basic level.
This code is for the original, not detective's mod which I will examine tomorrow unless Detective wants to apply the stuff below himself...
replace around line 100 the entire function pm_new_message
from:
function pm_new_message( $id_receiver, $id_sender, $pmtitle, $message ){
...
}to this:
function pm_new_message( $id_receiver, $id_sender, $pmtitle, $message ){
global $bbdb, $bb_table_prefix;
$created_on = bb_current_time('mysql');
$id_receiver = intval($id_receiver);
$id_sender = intval($id_sender);
$pmtitle=substr(strip_tags($pmtitle),0,64);
remove_filter('pre_post', 'post_regulation');
$message=substr($message,0,2048);
$message=force_balance_tags($message);
$message=apply_filters('pre_post',$message,0,0);
$message=apply_filters('post_text',$message,0);
$pmtitle=mysql_real_escape_string($pmtitle);
$message=mysql_real_escape_string($message);
$bbdb->query("INSERT INTO ".$bb_table_prefix."privatemessages
(id_sender, id_receiver, pmtitle, message, created_on)
VALUES
('$id_sender', '$id_receiver', '$pmtitle', '$message','$created_on')");
}This patch should in theory make it virtually completely sanitized (but still not completely validated unfortunately) and has the added bonus that most other plugins that affect posts like bb-smilies, etc. should work inside PM's
could you guys just please make private messaging part of the forum and NOT a plugin, man all this is really unprofessional.... hearing this news has just set me back so much !!
whats the point of having it as a plugin, and then a mod, and then security issues.... everyone adding bits to it, it becomes a mess.. Just make it part of the forum.
regards,
sway.
johnhiler
Member
It's because it is a plugin that we're all able to patch it without upgrading the entire bbPress install itself!
I agree that it'd be nice if the top plugins had security audits of some kind. Thank goodness a user reported this and another one fixed it though - that's exactly how open source communities should work (ideally). Thanks merlin214365 and _ck_!
I do feel dumb for just installing this plugin without even looking at the code. I'm going to do a personal security audit, and report back if any of the plugins I'm using turn up with security holes. Hopefully we can use this incident to improve the overall security of the most popular bbPress plugins!
_ck_
Moderator
Technically the plugin has been abandoned by the original author.
I don't think the download can be disabled but Sam or Michael might be able to move it on the SVN from the trunk directory to a tag where the extend section would make a link that would fail to download so that would prevent people from blindly installing it.
The plugin was written after bbPress .7 had just updated to .8 so there were few other plugins available for the author to model some security against. It's design is a little dated and spread out across several files makes it hard to maintain (a similar problem that the avatar upload plugin shares).
johnhiler
Member
Is it possible for someone to adopt the plugin... maybe Detective would be interested?
I'm just worried because 85 people downloaded the PM plugin this week! I know we can't do anything about those 85 users, but maybe we can do something to help next week's 85 downloaders?
_ck_
Moderator
If rewriting a PM plugin I would try to use as much internal stuff that bbPress has as possible. For example use the same hooks actions/filters that the new post form uses (and specify post id #0), which would make it work with many plugins. And run the message through pre_text before saving and then output through post_text when displaying.
It's also technically possible to do the whole thing inside of one single file for easy maintenance and make it part of the profile tabs. Look at bb-reputation to see the trick to making profile tabs within a single file, it requires a "wrapper".
johnhiler
Member
I guess we can't post a security warning on the PM plugin... I just gave it one star instead, in the hopes that a lower rating would discourage people from installing it.
http://bbpress.org/plugins/topic/private-messages/
Who wouldn't? I don't want PM in my forum. I prefer it in a plugin.
+1 chrishajer - PM was a way for people to stalk and harass each other on my old forums :/ 'ef it.
Now, perhaps a 'show/hide email' option would be nice, but PM? Ick. It makes you (the site owner) responsible for people being asshats in a way you can't easily monitor.
im not trying to be an ass people, i appreciate everything you guys have done, coz its the best light-forum on the net,,.... but a PM feature is available as standard on every forum, and it makes sense, to know if the feature is stable or not. and trust to download it. rather than rely on 3rd party pluging it.
johnhiler
Member
A possible compromise might be to have the PM functionality officially available (but as a plugin, not as part of the core). It would be written (or at least vetted) by bbPress staff... and even downloaded as part of the standard distribution. But each user could choose to enable or disable it, and it wouldn't be in the core.
I think we're getting close to that actually. BuddyPress has a PM plugin on its roadmap... maybe that can be adapted for bbPress.
Hmm, yeah someone should definitely try to get the current repository transferred to another person. I'm pretty new to bbPress so someone else would be a way better candidate than me for rewriting this plugin.
Btw, what did detective's mod add? I'm not even sure what I'm using...
Anyway while I was replacing pm_new_message with ck's mod I decided to hack email notification in there at the end (formatting stolen shamelessly from the existing notification plugin):
$to = bb_get_user_email($id_receiver);
$pm_link = bb_get_option('uri') . 'message.php?id=' . $bbdb->insert_id;
$message = __("You have a new private message: %1$s \nFrom: %2$s \n\n%3$s ");
mail( $to, bb_get_option('name') . ':' . __('Private Message'),
sprintf( $message, $pmtitle, get_user_name($id_sender), $pm_link ),
'From: ' . bb_get_option('admin_email')
);Yeah, it's definitely a quick and dirty temporary hack! I didn't even realize until now that admin_email has changed to from_email... oops! My forum's not so busy so I'm ignoring race conditions for now. In the future I might just rip off the bbpress post functions. Or wait for someone else to fix this plugin, heh! :p
citizenkeith
Member
If anybody is working on a new version of the PM plugin, please try to make it compatible with old messages... we've been using the current PM plugin for a while now, and would hate to lose all our previous correspondence.
Thanks for the warning and the security update, _ck_.
citizenkeith
Member
Ever since adding _ck_'s security update, we've been having a lot of problems with undelivered PMs. Last night I tried to send a forum member a PM about five times, and he never received it. After sending, I would scroll down to check my Sent Messages and none of them were there.
I'm running bbPress 0.9.0.3 with Private Messaging 0.80.
_ck_
Moderator
It's possible because I was testing it on 1.0 that 1.0 automatically escapes the mysql values passed and that 0.9 does not. I can't remember.
Try adding this before the $bbdb->query
$pmtitle=mysql_real_escape_string($pmtitle);
$message=mysql_real_escape_string($message);and see if it helps or if it just add slashes to your text instead and breaks it.
It may also be a quote problem but I thought the bbpress pre/post text filters should encode quote or any other problematic characters.
_ck_
Moderator
If the notification sends any part of the actual message, keep in mind the messages now contain html and may get rejected by spam filters. Other than that, I dunno.
Also you'd have to send the email notification BEFORE you do the escape if you send the message or title in the message (or save the original to different strings).
You must log in to post.