[NSIS-Bugs] [ nsis-Patches-2918870 ] Usage of the zlib compression library provided by the system

[SourceForge.net <no...@so...>]

Patches item #2918870, was opened at 2009-12-21 20:41
Message generated for change (Comment added) made by f0rt
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=373087&aid=2918870&group_id=22049

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: NSIS
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: f0rt (f0rt)
Assigned to: Amir Szekely (kichik)
Summary: Usage of the zlib compression library provided by the system

Initial Comment:
Programs statically linked to zlib impose extra work for security related fixes that are also needed for all the embedded copies of zlib.
The attached patch changes makensis so that it uses the zlib compression library provided by the system on POSIX systems.


----------------------------------------------------------------------

>Comment By: f0rt (f0rt)
Date: 2010-01-24 19:43

Message:
if you set the environment variable ZLIB_W32 specfying the location of zlib
development files for Windows in advance then the updated patch allows to
"build with just scons".

substart runs the executable of the same name in the 'Bin' sub folder and
passes along the command line options.
If you rename substart to makensis and place it in the root of the
installation folder then scripts expecting makensis in the root of the
installation folder continue to run as before. 

Comments, corrections and improvements are always welcome.

Below follows a list of the changes:
 Contrib/System/Source
System.h
-> asm renamed asmthunk because asm is a keyword of mingw/gcc

Contrib/VPatch/Source/GenPat
adler32.cpp SConscript
-> use adler32 checksum function from zlib shared respectively dynamic
link
   library



Contrib/zip2exe
SConscript ioapi.c unzip.c unzip.h
-> use functions from zlib shared respectively dynamic link library

adler32.c crc32.c crc32.h inffast.c inffast.h inffixed.h inflate.c
inflate.h
inftrees.c inftrees.h zconf.h zlib.h zutil.c zutil.h
-> deletion of obsolete files



Contrib/SubStart
ReadMe.txt SConscript substart.c
-> This tool runs the executable of the same name in the 'Bin' sub folder
and passes along the command line options.



Source (makensis)
SConscript czlib.h
-> use deflate functions from zlib shared respectively dynamic link
library

build.cpp makenssi.cpp
-> makensis moved to 'Bin' folder. Therefore the NSIS data directory
   is the parent directory of the directory

zlib/trees.c zlib/deflate.c
-> deletion of obsolete files



Source/Tests
SConscript
-> zlib dynamic library is installed before the tests are being run.


SConstruct
-> ZLIB_W32 environment variable respectively command line option
   for specifying the location of zlib development files for Windows.
   Detect makensis compiler in root and 'Bin' folder.


Docs/src/build.but
-> Updated documentation:
   * SCons version of at least 0.98 is required
     [EnsureSConsVersion(0,98) in SConstruct]
     -> maybe even a more recent version (I used scons 1.2.0)
   * NSIS build requires zlib.
   * Fixed broken link to Microsoft Visual C++ 6.0 Processor Pack



Examples/makensis.nsi
-> Put start redirector (susbstart) for makensis in the root of
   the installation folder
   Place makensis.exe and zlib dynamic link library in 'Bin' folder
   Delete manifest file during uninstall



SCons/utils.py
-> Added AddZLib function for detecting zlib and adding the
   appropriate compiler and linker options


----------------------------------------------------------------------

Comment By: Amir Szekely (kichik)
Date: 2010-01-07 20:59

Message:
Sounds like a good and well thought-of plan. Would clear up the code a bit
too. I only have a few things that don't sit well for me. The external
requirement for Windows build, ZLIB_W32 option and moving makensis[w].exe.

As for the first one, I guess we'll have to live with it as zlib doesn't
have SVN server we can svn:external on and we don't want to include the
source code even once because that was the whole point of this patch.

We already have a similar situation with wxWidgets and that uses an
environment variable. I think it would be nicer if it can stay consistent
and also simple to build with just `scons` and no options.

Moving the executables could break scripts for people that will be looking
at the old place. Only two solutions I can think of here are completely
dynamic linking (GetProcAddress) or stub executables that just pass on the
command line options to the Bin folder.

----------------------------------------------------------------------

Comment By: f0rt (f0rt)
Date: 2010-01-07 20:21

Message:
There are modifications in respect to the compressed data for the bzip2
compression. However the modifications to zlib don't affect the compressed
data with the exception of the header at least according to my analysis.

I would do the following tasks in order to deploy the zlib shared
respectively dynamic link library instead of the statically linked embedded
versions for the Windows and POSIX port :
* Add ZLIB_W32 build command line option for scons to specify the location
of the Win32 version of the zlib library
* Check for availability of header, import and dynamic link library of
zlib in build scripts [Win32 and/or POSIX]
  (If zlib header and library is installed on a POSIX system then normally
the compiler should be able to pick them up without the need to specify
their location.)
* Change installation path of makensis from the installation root into the
'Bin' subfolder so that the zlib1.dll put into the 'Bin' subfolder could be
accessed by zip2exe and GetPat as well.
* Adapt makensisw and zip2exe to run makensis from the 'Bin' subfolder
* Modify makensis, zip2exe and GenPat (uses adler32 checksum code) to use
shared zlib respectively dynamic link library
* Adapt makensis.nsi installation script to install makensis and zlib1.dll
into 'Bin' subfolder
* Update build.but to document the need of the zlib library and the
procedure how to specify the location of the installed zlib package for
building the NSIS package
* Remove duplicated and redundant source files which are no longer needed
because their functionality is handled via the zlib shared respectively
dynamic link library.


----------------------------------------------------------------------

Comment By: Amir Szekely (kichik)
Date: 2009-12-23 22:36

Message:
I remember I read Justin's modifications to zlib were in the data itself
too. But we have the unit tests for that and if they still pass, go ahead
and commit. Feel free to also update the Windows port and zip2exe in the
process.

----------------------------------------------------------------------

Comment By: f0rt (f0rt)
Date: 2009-12-22 22:30

Message:
The embedded zlib library in NSIS is optimized for size but it retains the
functionality of the original one.
The crucial difference is that by default the original zlib library emits
a header.  However the output of the header could be suppressed by using a
negative window size as a parameter for the deflateInit2 function of the
original zlib library. 

As an example for a security threat I'd like to mention DSA-112:
http://www.debian.org/security/2002/dsa-122

"The zlib vulnerability is fixed in the Debian zlib package version
1.1.3-5.1. A number of programs either link statically to zlib or include a
private copy of zlib code. These programs must also be upgraded to
eliminate the zlib vulnerability."

So there is extra work involved from a maintainer's point of view for each
package using its own private copy of zlib.

I used the following sample code to verify my findings. For test_z_nsis
the files from Source/zlib as well as the generated files config.h,
nsis-defines.h, nsis-version.h, nsis-sconf.h, Platform.h are needed.

/*
 gcc -g -Wall -o test_z test_z.c -lz
 ./test_z
 gcc -DNSIS -I. -g -Wall -o test_z_nsis test_z.c deflate.c trees.c
 ./test_z_nsis

 Output of both test_z and test_z_nsis:
 F3 48 CD C9 C9 57 08 CF 2F CA 49 51 04 00
 */
#include <stdlib.h>
#include <string.h>
#include <stdio.h>

#ifndef NSIS
#  include <zlib.h>
#  define streamInit(strm) { \
      strm->zalloc = (alloc_func)Z_NULL; \
      strm->zfree = (free_func)Z_NULL; \
      strm->opaque = (voidpf)Z_NULL; }
#else
#  include "ZLIB.H"
#  define deflateInit2(strm, level, method, windowBits, memLevel,
strategy) \
      deflateInit2_(strm, level, method, windowBits, memLevel, strategy,
\
      NULL, sizeof(z_stream))
#  define streamInit(strm)
#endif

int main(int argc, char *argv[])
{
   z_stream *stream = (z_stream *)malloc(sizeof(*stream));

   if (stream)
   {
      unsigned char hello[] = "Hello World!";
      unsigned char compressed[1024];
      int err;

      streamInit(stream);
      /* err = deflateInit(stream, 9); */
      err =  deflateInit2 (stream, 9, Z_DEFLATED, -MAX_WBITS,
MAX_MEM_LEVEL, Z_DEFAULT_STRATEGY);

      if (!err)
      {
         memset(compressed, 0, sizeof(compressed));
         stream->next_in = hello;
         stream->avail_in = sizeof(hello) - 1;
         stream->next_out = compressed;
         stream->avail_out = sizeof(compressed);
         err = deflate(stream, Z_FINISH);
         if (Z_OK == err || Z_STREAM_END == err)
         {
            unsigned int i;
            unsigned int n = (sizeof(compressed) - stream->avail_out);

            for (i = 0; i < n; i++)
            {
               printf("%02X ", compressed[i]);
            }
            printf("\n");
         }
      }
      free(stream);
   }
   return 0;
}



----------------------------------------------------------------------

Comment By: Amir Szekely (kichik)
Date: 2009-12-22 00:44

Message:
Are you sure this works? Our zlib library is a modified library that's
supposed to have different output format.

As for the security threat, we control both the compressor and the
decompressor. Any vulnerabilities in the decompressor are (a) not affected
by this patch and (b) require modification of the executable file which
already allows complete control.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=373087&aid=2918870&group_id=22049