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

[Soeren Gebbert]

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