Bug 691350

Well, you are right. The startup code has a rather unfortunate mode of
'search_with_no_combine' when gs_main_lib_open calls lib_file_open:
------------------------------------------------------------------------
    bool starting_arg_file = (i_ctx_p == NULL) ? true :
               i_ctx_p->starting_arg_file;
    bool search_with_no_combine = false;
---SNIP---
    if (gp_file_name_is_absolute(fname, flen)) {
       search_with_no_combine = true;
       search_with_combine = false;
    } else {
       search_with_no_combine = starting_arg_file;
       search_with_combine = true;
    }
    if (search_with_no_combine) {
        uint blen1 = blen;

        if (gp_file_name_reduce(fname, flen, buffer, &blen1) != 
                  gp_combine_success)
            goto skip;
        if (iodev_os_open_file(iodev, (const char *)buffer, blen1,
                                (const char *)fmode, &s, (gs_memory_t *)mem) == 
            0)
------------------------------------------------------------------------

I guess this is a real bug. At the very least we need to observe the
minst->search_here_first condition (as set by the -P- flag), but it is
not clear to me that we ever should search without combine, except for
the case when gp_file_name_is_absolute( ) returns true.

I'll look into this when I finish my travelling.

Due to the perceived gravity of the bug, the patch sent out for review a day ago is committed as r11352 . It was tested okay in combination with 691355/691356 before sending out for review:
http://bugs.ghostscript.com/show_bug.cgi?id=691355#c12
http://bugs.ghostscript.com/show_bug.cgi?id=691356#c5

Await feedback and possible refinement from other Artifex personnel before closing.

r11390 added short paragraph in man page about -P-/-P referring users to Use.htm for details.

Created attachment 6412 [details]
ghostscript-8.70-gs_init.dif

Suggest at least such a change to protect users against symlink
attacks within e.g. /tmp directory.  This is a solution for UNIX
based OS, don't know about Windows[tm].

Created attachment 6419 [details]
A slightly more portable version of ghostscript-8.70-gs_init.dif

A slightly more portable version of ghostscript-8.70-gs_init.dif, committed as r11468.

Thanks for the patch - I added a define and a comment about windows portability.
Not sure how this patch works under the emulated group/world permission on cygwin
on non-ACL win98/ME. It probably works but either way it is not important, as cygwin users can always run the native binary.

BTW, MS VC9 is freely available and works alright under wine with the little effort (http://bugs.winehq.org/show_bug.cgi?id=21394). For obviously unix-specific patches in the future like ghostscript-8.70-gs_init.dif, please make an effort to see to what extent it is non-portable.

Created attachment 6441 [details]
patch

Don't search for initialization files in the current directory first
by default because this leads to well-known security and confusion problems.
Do this only on the user's request by -P switch. Also revert rev. 11468,
which is no longer needed.

The patch has been committed as a rev. 11494.

(In reply to comment #9)
> Created an attachment (id=6441) [details]
> patch
> 
> Don't search for initialization files in the current directory first
> by default because this leads to well-known security and confusion problems.
> Do this only on the user's request by -P switch. Also revert rev. 11468,
> which is no longer needed.

The only problem is that curently even with -P- or SEARCH_HERE_FIRST=0
at build time the gs_init.ps in the current working directory will
be opened by gs first:

 /suse/werner> strace -o log -e open gs -P-
 GPL Ghostscript 8.70 (2009-07-31)
 Copyright (C) 2009 Artifex Software, Inc.  All rights reserved.
 This software comes with NO WARRANTY: see the file COPYING for details.
 GS>quit
 /suse/werner> grep gs_init.ps log
 open("gs_init.ps", O_RDONLY)            = -1 ENOENT (No such file or directory)
 open("/etc/ghostscript/8.70/gs_init.ps", O_RDONLY) = -1 ENOENT (No such file or directory)
 open("/usr/share/ghostscript/Resource/gs_init.ps", O_RDONLY) = -1 ENOENT (No such file or directory)
 open("/usr/share/ghostscript/8.70/Resource/Init/gs_init.ps", O_RDONLY) = 3
 /suse/werner> 

(the binary uses both the option -P- and was compiled with SEARCH_HERE_FIRST=0)
This looks like that the default for `i_ctx_p == NULL' should not be
true but `false'

Indeed the value of SEARCH_HERE_FIRST is used in the cpp macro
gs_main_instance_default_init_values within iminst.h and comes
with the const struct gs_main_instance_init_values within iconf.c
which becomes the default in gs_main_alloc_instance() and therefore
in gsapi_new_instance() and does set the default boolean value
of i_ctx_p->starting_arg_file which is not used if i_ctx_p is
NULL for gs_init.ps.

(In reply to comment #11)
>  /suse/werner> strace -o log -e open gs -P-
>  GPL Ghostscript 8.70 (2009-07-31)
                   ^^^^^^^^^^^^^^^^^

Please try SVN HEAD? (and close if it is to your satisfaction.).

It appears that comment #12 was from gs 8.70.

Closing since the recent (-r11494) changes are expected to have resolved this.

If this is not the case a NEW bug should be opened with a relevant description.

Created attachment 6449 [details]
ghostscript-gs_init.dif

This attachment are for those who have to support products
and/or distributions which include older ghostscript versions.
It is simply a backport of the current SVN version to gs <= 8.71.

While working on 691408, I found something that may lead to problems with 
rev 11494.

Reopening for discussion.

Here's the relevant section of the comment:

The problem is that with the -P- default condition (SEARCH_HERE_FIRST=0), the
'lib_file_open' logic gets called in order to load the file from
Resource/Encoding/Wingdings) and ends up taking the path at zfile.c:1009 that
sets search_with_no_combine = starting_arg_file; search_with_combine = true;
and since 'starting_arg_file' is now 0 (because of the -P- default) it does
not allow 'search_with_no_combine'.

Since GS_LIB=Resource/Init:lib it checks for the existence of:
"Resource/Init/Resource/Encoding/Wingdings"
"lib/Resource/Encoding/Wingdings"
and various other GS_LIB_DEFAULT paths.

Thus when -P- is the default (or SEARCH_HERE_FIRST=0 in the makefile), we
cannot find Resource files when the GenericResourceDir ends up as a relative
path (non-absolute).

Note that setting -sGenericResourceDir=Resource/ or ./Resource/ also does
not help this -- the file name is seen as not absolute, so does also only
searches for file names prepended with the lib paths.

Setting an absolute path works fine for me.
---SNIP---
This raises concern about the decision to make SEARCH_HERE_FIRST=0 as the
default for the future. It may cause a lot of folks to have problems
accessing Resource files when COMPILE_INITS=0

Some of the points to consider on this:

1. While GS_LIB=Resource/Init (or GS_LIB=./Resource/Init) works for finding
   the init files, it does not automatically generate a valid default
   GenericResourceDir since without -P (or -I.) Resource files cannot be
   opened.

2. Without -P or -I. -sGenericResourceDir must be set to an absolute path
   (even if the LIBPATH paths include relative paths).

3. When GenericResourceDir is set to a non-absolute path with -P- in effect,
   whether set by -sGenericResourceDir or by the 'default setting' code in
   gs_res.ps, it does not warn that the path is not valid. The testing in
   gs_res.ps doesn't use the same algorithm as ResourceFileStatus does:

   pssystemparams dup /GenericResourceDir get exch /GenericResourcePathSep get
   Init) exch (gs_init.ps) concatstrings concatstrings concatstrings 
   status { ...

I am really unsure about the wisdom of changing the default behavior. If linux
distros wish to do this (to address security concerns raised by this bug), they
can do so, AND TAKE RESPONSIBILITY FOR TESTING AND/OR CHANGING ANY SCRIPTS
that break as a result.

@Ray:  It is a simple security risk to allow reading configuration files
       from the working directory.  Think about /tmp or /var/tmp and
       a modified gs_init.ps ... or a simply tar or zip archive including
       such a modified gs_init.ps.  With this any user of gs will become
       an attack victim.  Maybe the search scheme should be able to detect
       if a file is part of the tree below /usr/share/ghostscript/<version>
       and accept any path below this even if not explicitly mentioned in
       the default GS_LIB path.

       For glibc based systems the way could be the clibc function call
       realpath(3) which is declared for (_BSD_SOURCE || _XOPEN_SOURCE >= 500)
       with the real path of /usr/share/ghostscript/<version> in comparision
       to any real path of a configuration file it is possible to determine
       if the last one is below to the first one.

Now that Alex has fixed the logic to set the default GenericResourceDir and
ICCProfilesDir values and insure that these directories are readable whether
they are in the %rom% file system or on disk, the builds seem to work fine
both in the default case (equivalent to the old -P- case) and with -P, also
with a -I added and deriving the default directories from there.

Closing as fixed. Note that the only way the default directory (.) will be
used now is if it is compiled into gs (which is no longer the default), or
if a user wants it to by including -P or by including a -I relative path.

All of this was completed by patches up to rev 11515.

Thanks for all of your work on this, Alex.

Also reported to Ubuntu and Debian, see

https://bugs.launchpad.net/gs-gpl/+bug/605211

-dSAFER will not be the default for rev 9.00. This parameter has been available
for several years and it is up to those that invoke Ghostscript with PS files to
take adequate precautions.

PDF has become dominant and doesn't have the general file control capability
that -dSAFER protects against.

Not that we don't recommend that folks use -dSAFER, we just want to make sure
that those that expect the current behavior to be surprised.

Hi Werner,Ray,

Could you provide a patch which includes complete fix for this bug? I'm supporting products which use ghostscipt 8.71 and not sure what modification should apply.

Appreciate your help very much!

Thanks,
Han