Re: Freepbx

[Tyler Borland <tborland1 () gmail com>]

Hello Marsh,

I had found one of the previous holes.
http://seclists.org/fulldisclosure/2010/Jul/180

Don't forget to check out the includes for that file.
http://www.freepbx.org/trac/browser/freepbx/trunk/amp_conf/htdocs/admin/cdr/lib/defines.php?rev=10274

On Tue, Sep 21, 2010 at 3:33 PM, Marsh Ray <marsh () extendedsubset com> wrote:

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