[ZOO-Discuss] Segfaults and security issues in ZOO WPS server

Tue May 25 05:17:29 PDT 2010

Hello Folks,
i have opened a ticket about the security and stability issues in the
zoo bug tracker:

http://www.zoo-project.org/trac/ticket/1

Criticizing is cheap, providing solutions is much better! :)
Hence a patch which fixes some minor issues is attached in the trac
bugtracker too.

I hope the patch can improve bit to the zoo kernel.

Best regards
Soeren

2010/4/26 Soeren Gebbert <soerengebbert at googlemail.com>:
> Hi Gérald,
>
> 2010/4/25 Gérald Fenoy <gerald.fenoy at geolabs.fr>:
>> Hi Soeren,
>> nice to hear from you, you're of great help.
>> I'm sorry for answering so late, I've explained to you on IRC today the bad
>> moments I spend during the end of this week :]
>> To go back to your message, in fact, I wondered when this kind of message
>> will arrived. Indeed I know that we have to correct lot of things in the
>> current code and I fully agree about your comments on code quality and
>> issues with it :)
>> Hopefully you're here to make us all remember this.
>> Please, don't hesitate to make your comments the way you want as you did in
>> the mail, I really appriciate it and I think that we have to be as critical
>> as we have to be more constructive.
>> As you noticed, this is the first ZOO Kernel release. This first release was
>> just a proof of concept for me and help us to define some rules for Services
>> Providers conception mainly. Now that we proven the concept works and it's
>> helpful for services providers developers, it's time to make it stable and
>> ready for a production use.
>> Nevertheless, I think that the step to make the code public have to be done
>> this way, as I think that the opensource community wait for us to make the
>> source code publicly available as an opensource software to use and test it.
>
> Agreed.
>
>> Now, it's the case.
>> Furthermore, in my opinion, lot of things we will need to be able to provide
>> a stable and robust solution are here already : the principle to parse a
>> metadata file, to dynamically load the shared object (from different
>> programming languages) / extract the service function from this last / run
>> the function passing parameters by reference (after converting arguments in
>> a specific language data type), then getting back the values to output them
>> as a ResponseDocument or in RawData format (after making the convertion in
>> the reverse order). So for me a big work has already been done.
>
> Well done, good work!
> Now, lets start first with the error response conception of the WPS
> server. IMHO we need a convenient logging and error response
> mechanism. This mechanism should create error XML response documents
> if an error occurs or in case of wrong or incomplete requests. Because
> ZOO-WPS supports many languages on shared library level, a convenient
> mechanism to catch segmentation faults or exit() calls in service
> libraries must be implemented, to assure correct error response. This
> response mechanism should be an outer hull over the WPS logic to catch
> any error.
> We can use the C++ try/throw/catch or the setjmp/longjmp C mechanism as basis.
>
> I would like to discuss this deeper in the zoo-project trac wiki, but
> unfortunately i am unable to add content or tickets there.
>
>> Moreover, devs won't have to modify their own Services Providers source code
>> to make them running on the new ZOO Kernel version. I think that is the
>> point : making people already able to develop their own Services Providers
>> using this first public release and then upgrade their ZOO Kernel
>> installation to the new one without having to recode their services.
>
> Thats a huge benefit of the server design.
>
>> Nevertheless, we thought about using YAML[1] rather than the ZCFG syntax we
>> defined, so devs should have to rewrite their ZCFG when they upgrade their
>> ZOO Kernel installation to use this new syntax. By the way, to avoid the
>> ZCFG rewrite, we will still support ZCFG files during some time, until we
>> get a converter from a ZCFG file to its equivalent YAML file available. This
>> way life of Services Providers coders will be made even easier.
>
> IMHO using YAML is a very good choice, thanks for this suggestion
> Gérald. It is standardized and there are C/C++ and python libraries
> for parsing available for many platforms. I would like to suggest to
> use the WPS process description standard (translated to YAML) for the
> process description part in the new YAML config files. If we use this
> standard as precise as possible (which should work with YAML) we can
> skip to think about our own YAML structure to represent the WPS
> process description content, but only about the zoo related content.
> Additionally this allows us to easily transform existing WPS XML
> document description into the YAML format.
>
> Best regards
> Soeren
>
>> Now that the first release is available, it is time to make it better !
>> Everybody which want to help in this step is welcome,
>> Regards,
>> [1] http://yaml.org/
>> Le 23 avr. 2010 à 01:36, Soeren Gebbert a écrit :
>>
>> Hello folks,
>> congrats to the first official release of the ZOO project and thanks
>> for the great work.
>>
>> I'm using the latest svn zoo kernel and services for GIS GRASS
>> integration on Suse linux 11.1 32bit.
>> While playing around with the ZOO WPS server i noticed some issues.
>>
>> The zoo_loader.cgi produces segfaults in the following cases:
>> * Parsing of the zcfg files seems to be case sensitive. My zcfg files
>> are currently hand-crafted, i.e: when i write by accident LIteralData
>> instead of LiteralData the loader crashes. I would like to suggest to
>> ignore the case in key word parsing, or to print an error
>> message/generating a response document in case a keyword is missing.
>> * When the metapath variable is missing in a request the loader crashes
>> i.e.:
>>  http://127.0.0.1/cgi-bin/zoo_loader.cgi?ServiceProvider=&Service=WPS&Request=GetCapabilities&Version=1.0.0
>>  instead of
>>  http://127.0.0.1/cgi-bin/zoo_loader.cgi?ServiceProvider=&metapath=&Service=WPS&Request=GetCapabilities&Version=1.0.0
>>  Is the ServiceProvider and the metapath parameter mandatory?
>>
>> Security and stability issues in the C source code:
>> * I noticed that many times strcmp is used for comparing strings. I
>> would like to suggest to use strncascmp instead for non case sensitive
>> parsing and for security reason.
>> * Memory allocation is many times not checked for success. To find
>> segfaults resulting from missing allocation checks is always a pain in
>> cgi scripts.
>> * Please avoid the use of sprintf. This C-function is the main reason
>> for critical security leaks and should IMHO not be used at all in
>> server applications. Segfaults resulting from sprintf created by
>> buffer over- or underflows are hard to find too. Always prefer
>> snprintf (_snprintf on windows) when creating strings.
>> * Why using the static function declaration in service.h and the
>> external declaration (which is the default) for local functions (i.e:
>> in service_internal.c, ...)?
>> * Some functions getting quite large and using redundant code
>> (service_internal.c)
>>
>> I would like to suggest the use of doxygen comment style for all
>> global and local functions for better documentation. The code will be
>> much better maintainable and external folks will be able to fix bugs
>> or enhance the core much easier.
>>
>> Establishing code reviews and refactoring sessions will largely
>> improve the code quality. :)
>>
>> I know this is the first official release and its just the beginning.
>> So please forgive me if i am too critical or offensive. IMHO the ZOO
>> WPS server will largely benefit from a secure and stable
>> implementation. Lets make th ZOO WPS server fit for production
>> environment. :)
>>
>> Best regards
>> Soeren
>> _______________________________________________
>> Zoo-discuss mailing list
>> Zoo-discuss at gisws.media.osaka-cu.ac.jp
>> http://gisws.media.osaka-cu.ac.jp/mailman/listinfo/zoo-discuss
>>
>> Gérald Fenoy
>> gerald.fenoy at geolabs.fr
>>
>> GEOLABS
>> Siège social :
>> Futur Building I
>> 1280, avenue des Platanes
>> 34970 Lattes
>> Tél. fixe : 04 67 53 67 37
>> Tél. portable : 06 70 08 25 39
>>
>