Freepbx

[Marsh Ray <marsh () extendedsubset com>]

Now, I'm no expert in PHP (don't even know it, really), and I didn't
test it myself, but these sure look like a mess o' SQL injections in
this here source file:

> http://www.freepbx.org/trac/browser/freepbx/trunk/amp_conf/htdocs/admin/cdr/call-comp.php?rev=10274

This code seems to define a list of variables which are accepted in the 
URL query string or as post variables. The function getpost_ifset is 
defined elsewhere:
line 8: getpost_ifset(array('current_page', 'fromstatsday_sday', 
'fromstatsmonth_sday', 'days_compare', 'min_call', 'posted',  'dsttype', 
'srctype', 'clidtype', 'channel', 'resulttype', 'stitle', 'atmenu', 
'current_page', 'order', 'sens', 'dst', 'src', 'clid', 'userfieldtype', 
'userfield', 'accountcodetype', 'accountcode'));

Line 104 seems to perform different action whether it's a POST or not:
if ($posted==1){

Line 260 suggests that $posted is actually determined by a variable, 
rather than the HTTP verb itself:
                <INPUT TYPE="hidden" NAME="posted" value=1>

Line 107 defines this 'do_field' function which apparently does little 
or no proper SQL escaping:
107       function do_field($sql,$fld){
108                     $fldtype = $fld.'type';
109                     global $$fld;
110                     global $$fldtype;
111             if (isset($$fld) && ($$fld!='')){
112                     if (strpos($sql,'WHERE') > 0){
113                             $sql = "$sql AND ";
114                     }else{
115                             $sql = "$sql WHERE ";
116                     }
117                     $sql = "$sql $fld";
118                     if (isset ($$fldtype)){
119                             switch ($$fldtype) {
120 case 1: $sql = "$sql='".$$fld."'";  break;
121 case 2: $sql = "$sql LIKE '".$$fld."%'";  break;
122 case 3: $sql = "$sql LIKE '%".$$fld."%'";  break;
123 case 4: $sql = "$sql LIKE '%".$$fld."'";
124                                                     }
125                     }else{ $sql = "$sql LIKE '%".$$fld."%'"; }
126                     }
127             return $sql;
128       }

This 'do_field' is called for several variables accepted from the client:
140       $SQLcmd = do_field($SQLcmd, 'clid');
141       $SQLcmd = do_field($SQLcmd, 'src');
142       $SQLcmd = do_field($SQLcmd, 'dst');
143       $SQLcmd = do_field($SQLcmd, 'channel');
145       $SQLcmd = do_field($SQLcmd, 'userfield');
146       $SQLcmd = do_field($SQLcmd, 'accountcode');

Some variables like 'days_compare' and 'fromstatsday_sday' seem to be 
singled out for particular unescapedness:

171 if (isset($fromstatsday_sday) && isset($fromstatsmonth_sday)) 
$date_clause.=" AND calldate < 
date'$fromstatsmonth_sday-$fromstatsday_sday'+ INTERVAL '1 DAY' AND 
calldate >= date'$fromstatsmonth_sday-$fromstatsday_sday' - INTERVAL 
'$days_compare DAY'";

So I'm not sure how much of a vulnerability disclosure this is. It's not 
even clear that the author intended to handle untrusted input. They may 
have intended everything under this "admin" subdirectory to be protected 
by some TLS or HTTP-level authentication.

However, I had a chance to play with a running setup a bit on Friday, 
and I was able to make requests to this URL as an unauthenticated user. 
(They took it down right before I was going to test it!) One might say 
it was a misconfiguration, but there have been similar advisories filed 
on this codebase in the past. It seems to be used under several products.

Of course, if POST variables are really accepted as GET query string 
parameters as well, that opens up a whole lot of XSS...

- Marsh

_______________________________________________
Full-Disclosure - We believe in it.
Charter: http://lists.grok.org.uk/full-disclosure-charter.html
Hosted and sponsored by Secunia - http://secunia.com/