Call Pickup (**EXT) security

I have run into an issue with call pickup security using **EXT and I felt the need to share my concerns with the FreePBX community and post a fix that I wrote. I don’t know the proper way to post code changes, so this seemed to be the best place to start. You can view more information about what led up to this here and, more recently, here. To summarize, ANYONE can pick up ANY RINGING EXTENSION using **EXT the way it is currently written.

The way this fix works, is it gets the devices callgroup and pickupgroup parameters when the user clicks the submit button on an extension, and inserts them into the internal asterisk database. the dialplan reads these attributes on an attempted ** call pickup and passes them to an outside python script which evaluates them and returns SUCCESS if the user is allowed to pickup that extension or APPERROR if they are not. The dialplan then takes care of the rest.

I am not a coder, but what I wrote worked for me. Here is what I did:

file: /var/www/html/admin/modules/core/functions.inc.php at line 675 of the original file.

original:

// Call pickup using app_pickup - Note that '**xtn' is hard-coded into the GXPs and SNOMs as a number to dial
// when a user pushes a flashing BLF. 
if ($fc_pickup != '') {
	$ext->addInclude('from-internal-additional', 'app-pickup');
	$fclen = strlen($fc_pickup);
	$ext->add('app-pickup', "_$fc_pickup.", '', new ext_NoOp('Attempt to Pickup ${EXTEN:'.$fclen.'} by ${CALLERID(num)}'));
	if (strstr($version, 'BRI')) 
		$ext->add('app-pickup', "_$fc_pickup.", '', new ext_dpickup('${EXTEN:'.$fclen.'}'));
	else
		$ext->add('app-pickup', "_$fc_pickup.", '', new ext_pickup('${EXTEN:'.$fclen.'}'));
}

modified:

// Call pickup using app_pickup - Note that '**xtn' is hard-coded into the GXPs and SNOMs as a number to dial
// when a user pushes a flashing BLF. 
if ($fc_pickup != '') {
	$ext->addInclude('from-internal-additional', 'app-pickup');
	$fclen = strlen($fc_pickup);
	$ext->add('app-pickup', "_$fc_pickup.", '', new ext_NoOp('Attempt to Pickup ${EXTEN:'.$fclen.'} by ${CALLERID(num)}'));
	//new code to do permission checking
	$ext->add('app-pickup', "_$fc_pickup.", '', new ext_setvar('CALLG','${DB(AMPUSER/${EXTEN:'.$fclen.'}/callgroup)}'));
	$ext->add('app-pickup', "_$fc_pickup.", '', new ext_setvar('PICKUPG','${DB(AMPUSER/${CALLERID(number)}/pickupgroup)}'));
	$ext->add('app-pickup', "_$fc_pickup.", '', new ext_system('${ASTAGIDIR}/parse.py ${CALLG} ${PICKUPG}'));
	$ext->add('app-pickup', "_$fc_pickup.", '', new ext_gotoif('$[${SYSTEMSTATUS} = APPERROR]','permden','permok'));
	$ext->add('app-pickup', "_$fc_pickup.", 'permden', new ext_playback('sorry-cant-let-you-do-that2'));
    $ext->add('app-pickup', "_$fc_pickup.", '', new ext_hangup(''));
//end new code
	if (strstr($version, 'BRI')) {
		//added from-did-direct and from-internal
		$ext->add('app-pickup', "_$fc_pickup.", 'permok', new ext_dpickup('${EXTEN:'.$fclen.'}'));
		$ext->add('app-pickup', "_$fc_pickup.", '', new ext_dpickup('${EXTEN:'.$fclen.'}@from-did-direct'));
		$ext->add('app-pickup', "_$fc_pickup.", '', new ext_dpickup('${EXTEN:'.$fclen.'}@from-internal'));
	}
	else {
		//added from-did-direct and from-internal
		$ext->add('app-pickup', "_$fc_pickup.", 'permok', new ext_pickup('${EXTEN:'.$fclen.'}'));
		$ext->add('app-pickup', "_$fc_pickup.", '', new ext_pickup('${EXTEN:'.$fclen.'}@from-did-direct'));
		$ext->add('app-pickup', "_$fc_pickup.", '', new ext_pickup('${EXTEN:'.$fclen.'}@from-internal'));
    }
                
}

In the same file, at line 2439 of the original file:
original:

//write to astdb
if ($astman) {
	$cid_masquerade = (isset($cid_masquerade) && trim($cid_masquerade) != "")?trim($cid_masquerade):$extension;
	$astman->database_put("AMPUSER",$extension."/password",isset($password)?$password:'');
	$astman->database_put("AMPUSER",$extension."/ringtimer",isset($ringtimer)?$ringtimer:'');
	$astman->database_put("AMPUSER",$extension."/noanswer",isset($noanswer)?$noanswer:'');
	$astman->database_put("AMPUSER",$extension."/recording",isset($recording)?$recording:'');
	$astman->database_put("AMPUSER",$extension."/outboundcid",isset($outboundcid)?"\"".$outboundcid."\"":'');
	$astman->database_put("AMPUSER",$extension."/cidname",isset($name)?"\"".$name."\"":'');
	$astman->database_put("AMPUSER",$extension."/cidnum",$cid_masquerade);
	$astman->database_put("AMPUSER",$extension."/voicemail","\"".isset($voicemail)?$voicemail:''."\"");

modified:

//write to astdb
if ($astman) {
	$cid_masquerade = (isset($cid_masquerade) && trim($cid_masquerade) != "")?trim($cid_masquerade):$extension;
	$astman->database_put("AMPUSER",$extension."/password",isset($password)?$password:'');
	$astman->database_put("AMPUSER",$extension."/ringtimer",isset($ringtimer)?$ringtimer:'');
	$astman->database_put("AMPUSER",$extension."/noanswer",isset($noanswer)?$noanswer:'');
	$astman->database_put("AMPUSER",$extension."/recording",isset($recording)?$recording:'');
	$astman->database_put("AMPUSER",$extension."/outboundcid",isset($outboundcid)?"\"".$outboundcid."\"":'');
	$astman->database_put("AMPUSER",$extension."/cidname",isset($name)?"\"".$name."\"":'');
	$astman->database_put("AMPUSER",$extension."/cidnum",$cid_masquerade);
	$astman->database_put("AMPUSER",$extension."/voicemail","\"".isset($voicemail)?$voicemail:''."\"");
	//added code to insert callgroup and pickupgroup info to the built in DB
	if (isset($devinfo_callgroup)) {
        $astman->database_put("AMPUSER",$extension."/callgroup",$devinfo_callgroup);
    }
    else {
        $astman->database_put("AMPUSER",$extension."/callgroup",'');
    }
    if (isset($devinfo_pickupgroup)) {
        $astman->database_put("AMPUSER",$extension."/pickupgroup",$devinfo_pickupgroup);
    }
    else {
        $astman->database_put("AMPUSER",$extension."/pickupgroup",'');
    }
    //end added code

I also had to write the following file and run chmod +x on it:
/var/lib/asterisk/agi-bin/parse.py:

#!/usr/bin/python

import string
import sys

# Get the values from asterisk
callgroup = sys.argv[1]
pickupgroup = sys.argv[2]

group_array = []

# split the string by commas
pgsplit = pickupgroup.split(',')

#hacky
j=0
i=0
while j < len(pgsplit):
  #If the value has a dash in it, the first statement will reject it sending it to the except: statement.
  try:
    group_array.append(int(pgsplit[j]))
  except:
    split_range = pgsplit[j].split('-')
    split_range[1] = int(split_range[1]) + 1
    range_count = range(int(split_range[0]), split_range[1])
    for num in range_count:
      group_array.append(num)
  j = j + 1


while i < len(group_array):
  if int(callgroup) == group_array[i]:
    sys.exit(0)
  i = i + 1

sys.exit(1)

I realize the whole thing is kinda hacky, but that’s how I got the pickup function to obey the callgroup and pickupgroup parameters. If you read the 2 links earlier in the post, you will see why the current **EXT call pickup method needs some work. I realize there are probably a few pieces missing in it, or better ways to do it, so feel free to contribute.

Sorry, I didn’t know to use the tracker. I posted a ticket here:
http://freepbx.org/trac/ticket/2814