Re: [Phpgroupware-developers] Testing CK-Ledger v.0.7.1 against phpgroupware-0.9.16.RC1

[C K Wu]

Hi, Dave,

Thank you for spotting the security flaw.  I'll
certainly tighten up security further.  At the same
time, '=' within the GET value string is a general
problem.  Any unstructured GET value string has the
potential of including a '=' char, thus causing
problem to the callee script.  Actually, this '='
problem could easily be rectified.  Instead of a
general 'split', class.sessions.inc.php could simply
pick on the 1st '=' char and assign the pre-'=' string
to GET arg and the post-'=' string to GET value. 
However, the '&' char could be much more difficult.  A
double urlencode/decode seems to be the easiest
solution.

Any thoughts ?

Cheers,
CK


Dave Hall :

>C K Wu <address@hidden> wrote:
>
>>Hello, folks,
>>
>>Me again.  There is some further complication with
the
>>urlencode/decode thing.
>>
>>In .../phpgwapi/inc/class.sessions.inc.php, around
>>line 1145, the code reads,
>>
>>/* Now we process the extravars into a proper url
>>format */
>>/* if its not an array, then we turn it into one */
>>/* We do this to help prevent any duplicates from
>>being sent. */
>>if (!is_array($extravars) && $extravars != '')
>>{
>>   $a = explode('&', $extravars);
>>   $i = 0;
>>   while ($i < count($a))
>>   {
>>       $b = split('=', $a[$i]);
>>       $new_extravars[$b[0]] = $b[1];
>>       $i++;
>>   }
>>   $extravars = $new_extravars;
>>   unset($new_extravars);
>>}
>>
>>Apparently, '=' is used as the GET argument/value
>>separator.  However, if there is a second '=' in
>>$a[$i], the value part will be truncated.  This is
the
>>case when an SQL is passed from script to script as
>>GET value.  In my case, the raw GET string looks
like
>>this,
>>
>>"filter= WHERE substring(source,1,2)='GD'"
>>
>>so the callee script only recovers $filter as "WHERE
>>substring(source,1,2)"
>
>
>Hmmmm.  I would argue that passing SQL via GET or
even POST is very poor
>security, and so it is good that this breaks.  Also
the $extravars, as
>shown in the code should be an array as it is faster
for process, no
>explode() needed :)
>


_________________________________________________________
最新鈴聲推介:遇見，亂世佳人，假如愛有天意...
http://ringtone.yahoo.com.hk