[Pkg-cas-maintainers] Fixing the /tmp usage and a bug in validate PGT()

[Raphael Geissert]

Hi,

On Tuesday 18 November 2008, Olivier Berger wrote:
> FYI, I have prepared another package (available at
> http://mentors.debian.net/debian/pool/main/l/libcas-php/libcas-php_1.0.1-2.
>dsc)
>
Ok, let's see.

>
> In order to address the following issues :
>
> On Tue, Nov 18, 2008 at 09:42:29AM +0100, Olivier Berger wrote:
>>
>> Le lundi 17 novembre 2008 à 15:20 -0600, Raphael Geissert a écrit :>>
>> > debian/rules:
>> > What about cleaning it up?
>> >
>> Sure.
>>
> done

I still see many commented-out lines, why?

>        dh_installman
>        dh_link
>        dh_strip

I don't see neither a manpage around nor a debian/links nor an ELF object.
If they are not used then don't call them.

>
>> > debian/copyright:
>> > > Upstream Author:
>> > >
>> > >          Pascal Aubry
>> >
>> > What about also displaying his email address?
>> Sure.
> done

> The Debian packaging is (C) 2008, Olivier Berger 
<olivier.berger at it-sudparis.eu> and
> is licensed under the GPL, see `/usr/share/common-licenses/GPL'.

You should better be more specific and say exactly what version of the licence 
you want.

>> >
>> > CAS.php:
>> > > define("CAS_PGT_STORAGE_FILE_DEFAULT_PATH",'/tmp');
>> > ..
>> > > define("CAS_PGT_STORAGE_FILE_FORMAT_PLAIN",'plain');
>> >
>> > Doesn't look good at all.
>>
>> Hmmm... I guess that needs to be fixed indeed. Thanks for spotting that.
>
> I have applied a patch in order to use /var/lib/libcas-php/pgtstorage/ and 
not /tmp for storage.

I'm not quite convinced that it is a good solution. But let's hold on for a 
moment on that problem (read below).

>
> Hope I did it in a safe way.
>
>
> In addition, I have tested more the proxy mode and fixed a nasty crash that 
occurred in validatePGT with the new domxml-php4-to-php5.
>

Good

>
> Any comments welcome

$ lintian -I -E libcas-php_1.0.1-2.dsc
I: libcas-php source: debian-watch-file-is-missing

And what about the api docs?

From CAS/client.php:
> function setPGTStorageDB($user,
...
> trigger_error('PGT storage into database is an experim...

If it is not supported then it should be documented and the dependency on 
php-db dropped or downgraded to suggests if you insist/think there are 
chances for it to be used.



I have not fully reviewed/audited the code, but the code has several 
vulnerabilities (symlink attacks, directory traversal, and XSS are those I 
have identified).

The symlinks attack can be launched because of predictable file names used and 
the default storage directory.
To make things worst, the user's input is not sanitized, so it is possible to 
predict the file name where data is going to be written to by passing an 
arbitrary pgtIou GET argument. The same lack of sanitization allows an 
attacker to either perform XSS or directory traversal attacks by abusing the 
callback function in CAS/client.php).

Additionally the functions calling getCallbackURL when proxy mode is enabled 
can lead to XSS attacks if the validation request fails. A similar situation 
also applies to functions calling getURL.

Tomorrow I'll send this information to bugtraq and will file the corresponding 
bug reports against packages shipping phpCAS.

I strongly recommend you and upstream to audit the code.

>
> Best regards,

Cheers,
-- 
Raphael Geissert - Debian Maintainer
www.debian.org - get.debian.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://lists.alioth.debian.org/pipermail/pkg-cas-maintainers/attachments/20081120/1dc19559/attachment.pgp