The following function only check if the current fbu have already mapped a local account. But it does not check if the local account already mapped with another fbu. As A result, by default, the fbu will be over written with the current logged facebook acount. I have user's account get stolen because of this!!! they log in my site from a public computer (cafe nearby, always use by our staff) and then logged out, not knowing that the public computer already logged a thief's facebook account. and the theif fbu overwritten the victim fbu when my staff login. The thief then log in my web site using his facebook account which already connected to the victim account and he changed the victim local password. And victim can never log in again. This is a very serious security hole..No just security, it accidentally remove some user of our staff account mix them up!!!! Very serious !!! 

<?php
function _fb_user_create_map() {
	global $user;
  if ($GLOBALS['user']->uid) {
	$fbu = fb_facebook_user();
        $account = fb_user_get_local_user($fbu);	

	if ($fbu && !$account && 
        !fb_controls(FB_USER_CONTROL_NO_CREATE_MAP)) {
				debugr("OH My God");
      _fb_user_set_map($GLOBALS['user'], $fbu);
      fb_invoke(FB_USER_OP_POST_USER_CONNECT, array(
                  'account' => $GLOBALS['user'],
                  'fbu' => $fbu,
                ), NULL, 'fb_user');
      return TRUE;
    }
  }
  return FALSE;
}
?>

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Because this module does not have a final, supported stable release (e.g. a 7.x-1.0) this issue can be discussed in public even though, in general, the Drupal Security Team expects security issues to be reported confidentially.

See http://drupal.org/security-advisory-policy and http://drupal.org/node/101494 for more information.

kris_mcl’s picture

kris_mcl commented
Project: Facebook Connect » Drupal for Facebook

This was posted in the issue queue for the FB Connect module. However, that code doesn't appear anywhere in the fb_connect module, but instead in the Drupal for Facebook module (http://drupal.org/project/fb).
Hopefully it'll get more response in the proper queue.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

The 6.x-3.x and 7.x code has already been using that constant since at least September 2010.

I can't find that code in the 6.x-2.x version. Can you clarify where you found this?

boftx’s picture

boftx commented

Does this affect 6.x-3.1?

13rac1’s picture

13rac1 commented
Status: Active » Postponed (maintainer needs more info)
greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Postponed (maintainer needs more info) » Active

What information is needed?

Dave Cohen’s picture

Dave Cohen commented
Title: Serious Risk Big bug for FB_USER_OPTION_MAP_ALWAYS » Drupal account could get mapped to wrong facebook account
Version: 6.x-2.x-dev » 6.x-3.x-dev

I think he was referring to your questions in #3.

If I understand this correctly, the following can happen on a site where fb_user.module is not configured to create accounts automatically, but is configured to map accounts.

A "thief" can log into facebook. Thief then uses facebook connect on your drupal site, but does not register on drupal site.

Thief walks away from the machine, leaving themselves logged into facebook and not logged into drupal site.

Victim uses the machine. Does not visit facebook.com. Does visit drupal site and registers/logs in.

At this point drupal for facebook thinks, this user is logged into drupal and also facebook, so it maps the two accounts.

Victim logs out of drupal and leaves.

Thief logs into facebook, returns to drupal site and finds herself logged into the victims account.

My first idea to prevent this is when fb_user.module alters the register/login forms, it needs a "not you?" link next to the facebook profile picture. If clicked that link would log user out of facebook. Is that right? Any better ideas?

13rac1’s picture

13rac1 commented

Yes. Status was changed for #3.

carl.zata’s picture

carl.zata commented
Title: Drupal account could get mapped to wrong facebook account » Drupal account get mapped to wrong facebook account
Version: 6.x-3.x-dev » 7.x-3.3-beta3

Hi,

i'm currently getting this error. Accounts in our site are getting mapped with wrong facebook accounts.
We are using 7.x-3.3-beta3 version of the module. any one can help us on this?

carl.zata’s picture

carl.zata commented
Issue summary: View changes

changed some wordings

Yuri’s picture

Yuri commented

I would like to know if the security hole mentioned in #7 is currently still present, if so it should get serious attention