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 <[email protected]> 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/
