On Wed, Jul 12, 2006 at 01:28:45AM +0200, Viktor Griph wrote: > On Tue, 11 Jul 2006, Dominik Vogt wrote: > > >On Tue, Jul 11, 2006 at 10:16:09AM -0500, fvwm-workers wrote: > >>CVSROOT: /home/cvs/fvwm > >>Module name: fvwm > >>Changes by: griph 06/07/11 10:16:09 > >> > >>Modified files: > >> . : ChangeLog NEWS configure.ac > >> modules : ChangeLog > >> modules/FvwmCommand: FvwmCommand.1.in FvwmCommand.c > >> FvwmCommandS.c fifos.c > >> > >>Log message: > >>fix tempfile vulnerabilities in FvwmCommand (bug #2791). > > > >Can you explain what you actually did, please? > > > > Sure. > First: When deciding on the default path the three files that are to be > used are tested with lstat (or stat if lstat is unavalable) to have the > same owner as the process owner, not have nore than one hard link and not > be a directory nor a symbolic link. If any of the tests fail the path will > be redirected to $FVWM_USERDIR instead of /var/tmp to avoid attacks > blocking the module. If some tests are impossible to do they are > concidered OK.
Actually, since we are creating the file with O_CREAT | O_EXCL, there is no chance of a symlink attack (because symlinks "exist" even if they do not point to anything). > Second: All open() calls use O_NOFOLLOW if that flag is defined. Good, but I don't want new ifdefs in the code. Instead, please add this to the end of the AH_VERBATIM macro "_ZEND_EXPLICIT_DEFINITIONS" in configure.ac, beginning at line 1195: #ifndef O_NOFOLLOW #define O_NOFOLLOW 0 #endif This snippet is added to the end of config.h. Afterwards you can use O_NOFOLLOW without ifdefs. As a rule of thumb, never put any ifdefs in a .c file (or better: anywhere). > I believe this should be enough, We really shouldn't be using public tempdirs at all. I wasn't aware that we are using /var/tmp. > but if one are really paranoid one could > add checks of the opened files in FvwmCommand.c to verify that they are > fifos with correect permissions. Ah, but it's too late if the file is open already. Ciao Dominik ^_^ ^_^ -- Dominik Vogt, [EMAIL PROTECTED]
signature.asc
Description: Digital signature
