372734 - File descriptor are not closed in child process

Status: RESOLVED DUPLICATE of bug 147659

(Core :: Networking: File, defect)

Description

[Martin Gerbershagen]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de; rv:1.8.0.10) Gecko/20070223 Firefox/1.5.0.10
Build Identifier: Mozilla Thunderbird 1.5.0.10

If a child process is forked from thunderbird, it inherits the open file descriptors from its parent. This happens for example, if an attached picture is viewed using a different program. On my system the program display (part of ImageMagick) is called to view a JPEG picture. The display process has nearly the same file descriptor open as thunderbird. This is the output of lsof:

mr@ger04:/users/mr> lsof -p 5536
COMMAND  PID USER   FD   TYPE     DEVICE    SIZE     NODE NAME
display 5536   mr  cwd    DIR        3,2   10584      840 /users/mr
display 5536   mr  rtd    DIR        3,5     656        2 /
display 5536   mr  txt    REG        3,6   10786    11717 /usr/bin/display
display 5536   mr  mem    REG        3,5  112347      121 /lib/ld-2.3.2.so
display 5536   mr  mem    REG        3,6   11498  4121505 /usr/X11R6/lib/X11/locale/lib/common/xlcDef.so.2
display 5536   mr  mem    REG        3,6   37247  4511635 /usr/lib/ImageMagick-6.3.2/modules-Q16/coders/jpeg.so
display 5536   mr  mem    REG        3,6   41404    17624 /usr/X11R6/lib/libXcursor.so.1.0
display 5536   mr  mem    DEL        0,5         20676614 /SYSV00000000
display 5536   mr  mem    REG        3,6 1959793  3541804 /usr/lib/libMagick.so.10.0.7
display 5536   mr  mem    REG        3,6  836193    56625 /usr/lib/libWand.so.10.0.7
display 5536   mr  mem    REG        3,6  152651    15178 /usr/lib/liblcms.so.1.0.10
display 5536   mr  mem    REG        3,6  367646   774620 /usr/lib/libtiff.so.3.8.2
display 5536   mr  mem    REG        3,5 1461208      118 /lib/i686/libc.so.6
display 5536   mr  mem    REG        3,6  142078    11658 /usr/lib/libjpeg.so.62.0.0
display 5536   mr  mem    REG        3,6   65383    47286 /usr/X11R6/lib/libXext.so.6.4
display 5536   mr  mem    REG        3,6  376585    47302 /usr/X11R6/lib/libXt.so.6.0
display 5536   mr  mem    REG        3,6   71364    11367 /usr/lib/libbz2.so.1.0.0
display 5536   mr  mem    REG        3,5   77388    13415 /lib/libz.so.1.2.3
display 5536   mr  mem    REG        3,5   80714      120 /lib/i686/libpthread.so.0
display 5536   mr  mem    REG        3,6  491556  2469501 /usr/lib/libfreetype.so.6.3.10
display 5536   mr  mem    REG        3,5   13625      130 /lib/libdl.so.2
display 5536   mr  mem    REG        3,6   38588    47279 /usr/X11R6/lib/libSM.so.6.0
display 5536   mr  mem    REG        3,6   96131    17612 /usr/X11R6/lib/libICE.so.6.3
display 5536   mr  mem    REG        3,6 1108503    47281 /usr/X11R6/lib/libX11.so.6.2
display 5536   mr  mem    REG        3,5  183008      119 /lib/i686/libm.so.6
display 5536   mr  mem    CHR        1,5            12678 /dev/zero
display 5536   mr  mem    REG        3,6   33002    47300 /usr/X11R6/lib/libXrender.so.1.2
display 5536   mr  mem    DEL        0,5         20611079 /SYSV00000000
display 5536   mr  mem    DEL        0,5         20643848 /SYSV00000000
display 5536   mr    0r   CHR        1,3             7650 /dev/null
display 5536   mr    1w   REG        3,2    4806    38979 /users/mr/.xsessionlog
display 5536   mr    2w   REG        3,2    4806    38979 /users/mr/.xsessionlog
display 5536   mr    3u  unix 0xca4d6040           409780 socket
display 5536   mr    4w   REG        3,2       0   140897 /users/mr/.thunderbird/iith3zih.default/.parentlock
display 5536   mr    5w   REG        3,2       0   140897 /users/mr/.thunderbird/iith3zih.default/.parentlock
display 5536   mr    7r  FIFO        0,6           409099 pipe
display 5536   mr    8w  FIFO        0,6           409099 pipe
display 5536   mr    9r  FIFO        0,6           409100 pipe
display 5536   mr   10w  FIFO        0,6           409100 pipe
display 5536   mr   14r  FIFO        0,6           409067 pipe
display 5536   mr   15w  FIFO        0,6           409067 pipe
display 5536   mr   16r  FIFO        0,6           409068 pipe
display 5536   mr   17w  FIFO        0,6           409068 pipe
display 5536   mr   18r  FIFO        0,6           409069 pipe
display 5536   mr   19w  FIFO        0,6           409069 pipe
display 5536   mr   21r  FIFO        0,6           409115 pipe
display 5536   mr   22w  FIFO        0,6           409115 pipe
display 5536   mr   23r  FIFO        0,6           409116 pipe
display 5536   mr   24w  FIFO        0,6           409116 pipe
display 5536   mr   25r  FIFO        0,6           409117 pipe
display 5536   mr   26w  FIFO        0,6           409117 pipe
display 5536   mr   27r   REG        3,7  860362    95798 /opt/thunderbird/lib/chrome/classic.jar
display 5536   mr   28r   REG        3,7 1697889    96469 /opt/thunderbird/lib/chrome/toolkit.jar
display 5536   mr   29r   REG        3,7  575637    96582 /opt/thunderbird/lib/chrome/de.jar
display 5536   mr   30u   REG        3,2    4059    83958 /users/mr/.thunderbird/iith3zih.default/panacea.dat
display 5536   mr   31r   REG        3,7 2652159    96517 /opt/thunderbird/lib/chrome/messenger.jar
display 5536   mr   32r   REG        3,7 1586835    96498 /opt/thunderbird/lib/chrome/comm.jar
display 5536   mr   33r   REG        3,7  790405    65672 /opt/thunderbird/lib/extensions/{847b3a00-7ab1-11d4-8f02-006008948af5}/chrome/enigmail.jar
display 5536   mr   34r   REG        3,7   26955    65655 /opt/thunderbird/lib/extensions/{847b3a00-7ab1-11d4-8f02-006008948af5}/chrome/enigmail-skin-tbird.jar
display 5536   mr   35r   REG        3,7   76802   187588 /opt/thunderbird/lib/extensions/{CC3C233D-6668-41bc-AAEB-F3A1D1D594F5}/chrome/mailredirect.jar
display 5536   mr   36r   REG        3,7    9510   187585 /opt/thunderbird/lib/extensions/{CC3C233D-6668-41bc-AAEB-F3A1D1D594F5}/chrome/mailredirect-skin.jar
display 5536   mr   37r   REG        3,7   10679    96464 /opt/thunderbird/lib/chrome/offline.jar
display 5536   mr   38r   REG        3,7  139291    96462 /opt/thunderbird/lib/chrome/newsblog.jar
display 5536   mr   39u   REG        3,2    1395    83930 /users/mr/.thunderbird/iith3zih.default/history.mab
display 5536   mr   40u   REG        3,2    1161    83971 /users/mr/.thunderbird/iith3zih.default/Mail/pop/Templates.msf
display 5536   mr   41u   REG        3,2    1290    83972 /users/mr/.thunderbird/iith3zih.default/Mail/pop/Junk.msf
display 5536   mr   42u   REG        3,2    1290    83993 /users/mr/.thunderbird/iith3zih.default/Mail/ger04/Junk.msf
display 5536   mr   45u   REG        3,2  102865    83986 /users/mr/.thunderbird/iith3zih.default/Mail/ger04/Inbox.msf
display 5536   mr   46u   REG        3,2   17151    83929 /users/mr/.thunderbird/iith3zih.default/abook.mab
display 5536   mr   48w  FIFO        0,6           409704 pipe
display 5536   mr   49r  FIFO        0,6           409219 pipe
display 5536   mr   50w  FIFO        0,6           409219 pipe
display 5536   mr   52w  FIFO        0,6           409705 pipe
display 5536   mr   54u   REG        3,2   19797    83964 /users/mr/.thunderbird/iith3zih.default/Mail/pop/Sent.msf

The pipes are the same as in the thunderbird process. This behaviour is very insecure. A malicious child process could write something to the pipes and then disturb the parent process. Any open sockets can also be accessed from the child process, which could access an existing imap connection without authentication.


Reproducible: Always

Steps to Reproduce:
1. Open a mail with an attached JPEG picture.
2. Open the attachment with any viewer.
3. lsof the viewer program.
Actual Results:  
Many files, pipes and sockets opened by thunderbird are also open in the child process.

Expected Results:  
The child process should only open its own files. The file descriptors inherited from the parent process should be closed between the fork and the exec call in the child process. In addition the socket options "close in exec, FD_CLOEXEC" should be set for all sockets.

I assume that the problem is also in firefox and seamonkey. The problem is also present on a Solaris system, so I think on any UNIX/Linux system.

Comment 1

[Martin Gerbershagen]

Attachment: Proposed patch t osolve the problem

The proposed patch solves the problem. The code works under hp-ux, linux and solaris.

Comment 2

[Daniel Veditz [:dveditz]]

This would presumably apply to Firefox as well since these routines are shared.

Comment 3

[Wan-Teh Chang]

This bug is a known problem.  It should be marked as a duplicate of bug 147659.
This problem was most recently reported in bug 368003.

Unfortunately I can't change NSPR's ForkAndExec function this way.  Mozilla
will have to:
- mark all the files it opened close-on-exec, or
- use its own spawn-process function that closes all the open file descriptors
  in the child process

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

As I was reading this bug, I kept thinking: why do they think this is a 
fault of NSPR.  Surely this is the reason for close-on-exec.  The opener 
of the file should set that flag.  Under some circumstances, keeping the 
FD open in the child may be the desired & intended behavior, and having 
NSPR close all the FDs would defeat that.  File openers should set 
close on exec.  (I have often thought that should be the default for 
FDs other than 0, 1 and 2.)

Comment 5

[Martin Gerbershagen]

Sorry I disagree with the previous comment. A child usually does not know, what the correct file descriptor is in the parent for a particular purpose. If you want that, you have to pass additional information to the child. I can't imagine any case except of self forking where this is needed. As soon as you do an exec the child should open its own files and sockets. Also to set the close on exec for all open files is not feasable in practice. For example if you open a file using the C++ *fstreams, you can't access the file descriptor. The only reasonable way is to close them before the exec call in the child process. BTW I am running the proposed patch since more than 1 week without any problems.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

NSPR has a mechanism rather similar to close-on-exec.  It allows open
PRFileDesc's to be marked as "inheritable" or not.  There is production 
software that depends on that feature.  I'm quite sure that the
patch shown above breaks that feature, closing fds whether they were
marked inheritable or not.  

Comment 7

[Wan-Teh Chang]

Martin, your patch would be appropriate if the NSPR library
were only used by the Mozilla clients.  The NSPR library is a
general-purpose library used by many other products.  This is
why we can't check in your patch.

Mozilla will need to write a new function to implement your
proposed fix.

Comment 8

[Daniel Veditz [:dveditz]]

wanted on the branches, but not blocking. If we get a trunk fix we can consider taking it. Since NSPR doesn't want this change this should go back to being a Thunderbird bug. Not duping to bug 147659 because some of the comments there tried to limit the scope to the browser.

Comment 9

[Martin Gerbershagen]

It is not a problen for me, if the patch does not go int other NSPR as proposed. It is ok for me, if you write another function that contains the close fds stuff to fork off a subprocess and and use the new function to invoke the helper applications. But keep in mind that closing all open fds except std{in|out|err} before the exec call should be the default behaviour. To me not closing them is only useful in exceptional cases. The correction should also be inserted into a central library, because both firefox and thunderbird need it and I assume also seamonkey.

Comment 10

[chris hofmann]

is this still a problem on the trunk?  should the component be changed as dan suggestion in commment 2 ?

Comment 12

[Mark Banner (:standard8)]

Whichever way I look at this, this seems to be a core bug, not Thunderbird specific.

Comment 13

[Karl Tomlinson (:karlt)]

As noted in bug 147659 comment 41, F_CLOEXEC is not a solution for threaded applications.

(In reply to comment 3)
> Unfortunately I can't change NSPR's ForkAndExec function this way.

Yes, but would an approach that only closed fds not explicitly marked as inheritable be acceptable?  AIUI the default behavior for fds not marked inheritable already differs across platforms?