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

[Dave Hall]

C=20K=20Wu <address@hidden> wrote:

> Hi, Dave,
> 
> Thank you for spotting the security flaw.  I'll
> certainly tighten up security further.  

No problem.  Good to hear :)

> 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 ?

Pass the $extravars arg of the link() method an array, not a string. 
That is how it is designed to be used.  So then = and & are properly
encoded, along with all other values.  I this is a little bit of work,
but it will make it run faster, as no explode()ing is done on the
$extravars variable within the link() method, it just url_encode()s it.

Hope this helps

Cheers

Dave

> 
> 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 :)
> >
> 
> 
> _________________________________________________________
> ³Ì·s¹aÁn±À¤¶:address@hidden
> http://ringtone.yahoo.com.hk
> 
>

dave.hall.vcf
Description: Card for <dave.hall@mbox.com.au>