SECURITY WARNING: stop using Private Messaging plugin
-
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.
-
That’s really sad. I suppose i was too naive when i modded the plugin. In fact, looking at the code now, it really screams “hack me”. There are almost no validations in some critical operations.
It’s worse than validations.
You improved a few areas but the main problems are still wide open.
I think I can post a temporary fix soon but it’s still not completely safe.
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
Remove it from the plugin-list
Thank you _ck_!
I edit this.
I’ll fix the modded plugin this weekend (i also plan to rewrite some things). Thanks for the fix.
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.
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!
Also: Having it be a plugin means that people who don’t wont those ‘features’ don’t have to disable. It’s the basic idea of bbPress: Simple
I just noticed that the unpatched PM plugin is still available in the plugins section, where it’s listed as one of the most popular downloads.
Is it possible to disable the download for now, until the code is updated with the security patch?
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).
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?
I’m interested (in a complete rewrite). I was going to wait for 1.0 beta to start a “private discussions” plugin. But seeing the current situation i’ll try to start sooner.
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”.
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.
who wouldnt want PM in a forum anyway.
its like if you the PM feature part of BBPress, than you could update it easier with the newer releases. it doesnt make sense , but anyway..
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.
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 nn%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')
);That’s a good hack Ann but keep in mind
insert_id
isn’t always reliable.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
Yeah unfortunately they didn’t even bother to maintain backward compatibility on the email change (which kinda annoyed me) so to support 0.8 you should first check from_email and if that is empty use admin_email.
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_.
- You must be logged in to reply to this topic.