Skip to:
Content
Pages
Categories
Search
Top
Bottom

SECURITY WARNING: stop using Private Messaging plugin


  • _ck_
    Participant

    @_ck_

    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.

Viewing 25 replies - 1 through 25 (of 41 total)

  • Detective
    Member

    @detective

    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.


    _ck_
    Participant

    @_ck_

    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.


    _ck_
    Participant

    @_ck_

    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


    Shagalaga
    Member

    @shagalaga

    Remove it from the plugin-list

    Thank you _ck_!

    I edit this.


    Detective
    Member

    @detective

    I’ll fix the modded plugin this weekend (i also plan to rewrite some things). Thanks for the fix.


    swaymedia
    Member

    @swaymedia

    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

    @johnhiler

    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 :)


    johnhiler
    Member

    @johnhiler

    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?


    _ck_
    Participant

    @_ck_

    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

    @johnhiler

    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?


    Detective
    Member

    @detective

    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.


    _ck_
    Participant

    @_ck_

    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

    @johnhiler

    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/


    swaymedia
    Member

    @swaymedia

    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..


    chrishajer
    Participant

    @chrishajer

    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.


    swaymedia
    Member

    @swaymedia

    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

    @johnhiler

    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.


    superann
    Member

    @superann

    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')
    );


    _ck_
    Participant

    @_ck_

    That’s a good hack Ann but keep in mind insert_id isn’t always reliable.


    superann
    Member

    @superann

    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


    _ck_
    Participant

    @_ck_

    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.


    citizenkeith
    Participant

    @citizenkeith

    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_.

Viewing 25 replies - 1 through 25 (of 41 total)

You must be logged in to reply to this topic.