Re: lynx-dev Lynx 2.8.3 with nmh problems

[T.E.Dickey]

> 
> > > So lynx stopped working in some situations, where earlier versions  
> > > did work.  There is no dunctional reason for it.  It makes sense for  
> > > some program (mhnshow, metamail) that can invoke arbitrary helpers  
> > > and (directly or indirectly) save arbitrary files to disk to disable  
> > > the 'x' bit; or at least, I can see the logic.  This shouldn't make  
> > > lynx fail.  
> >  
> On Mon, 8 May 100, T.E.Dickey wrote: 
> > we can make it more fool-proof by repairing umask (otherwise any 
> > directories that lynx creates in dired mode would be unusable as well) 
>  
> I didn't think about dired; but I think we shouldn't "repair umask". 
> If lynx gets invoked with a certain umask, in general the assumption 
> should be that that's what is desired. 

I had in mind ensuring that if read permission is not masked, execute
shouldn't be either (a simple mask/shift/mask operation).  With Unix
permissions set up as they are, that's an got to be an error.
  
> > the temp subdir is the piece that I needed to work around the code that 
> > appends to and reuses temp-files 
>  
> Which code do you mean?  LYOpenTempRewrite, or something else? 

iirc, yes.
  
>   - in LYOpenTempRewrite().  This is safe, since LYReopenTemp 
>     is only called if the file exists and we have verified that it 
>     'IsOurFile'. 
But IsOurFile is really only 100% reliable for cases where there's no race.
  
> In other places, the file being appended is not a temp file in the 
> temp space (LYBookmark.c), or the code is dead [LYPrint.c: (TOUPPER(c) == 
> 'A') 
> can't be true]. 

I wasn't convinced of that since the same low-level permissions code is
called.
  
> > (those are not secure in the sense that 
> > the recent complaints about temp-file manipulation don't need any other 
> > examples). 
>  
> I don't understand this sentence.  Please rephrase... 

The people who were criticizing the temp-file manipulation only had to
point out the problems in making reopening a temp-file to make their point.
  
> > I could relax the check a little if I didn't know that some machine's 
> > t-bit doesn't prevent renaming of files (not iirc directories though). 
>  
> Well, not on this machine.  Not in most sane environments, right? 

older machines (lynx users seem to favor those ;-)

> > then the mkdir will fail and/or the permissions and ownership check will 
> > fail - but lynx won't create other files. 
>  
> Which checks do you mean - the ones preceding mktemp/mkdir, or later 
> on (in LYUtils.c)? 

no - the check after creating the directory to ensure that the newly
created directory is really owned by "us".  (I have some refinements for
this which I noticed with xterm, but it won't change the design much).
  
> > > Or attacker could modify the underlying directory (the original  
> > > 'lynx_temp_space').  That either will make the mkdir() fail - opening  
> >  
> > no - mktemp only makes a name - doesn't touch the filesystem 
>  
> I see the following system calls generated from 
>  
>             if (mktemp(lynx_temp_space) == 0 
>              || mkdir(lynx_temp_space, 0700) < 0) { 
>  
> (TMPDIR is "/usr/src/tmp" here): 
>  
>    gettimeofday({957816451, 396599}, NULL) = 0 
>    getpid()                                = 25972 
>    stat("/usr/src/tmp/Rc8kfD", 0xbffff660) = -1 ENOENT (No such file or 
> directory) 
>    mkdir("/usr/src/tmp/Rc8kfD", 0700)      = 0 
>  
> The stat is from mktemp().  It doesn't "touch" the filesystem in the 
> sense of writing anything, but it has to check whether a file exists. 

yes - so there's a race there.
but the return value of mkdir should be sufficient to verify that.
  
> > The mkdir is the only operation that has to succeed or fail (if it already 
> > exists, it will fail;  
>  
> ... thereby giving an attacker a way to prevent my lynx process from 
> running - if he can guess the filename (just one) for the directory 
> and create it at the right moment. 
>  
> Can you see why I don't want this code, or want a way to suppress it? 

no - it's simple to set $TMPDIR to a directory which satisfies lynx,
even one which you create yourself under /tmp.
  
> After all, I might be running lynx with -dump or -source, from a cron 
> job or cgi script etc., such that no temp file will ever be generated. 

ok - that's a reasonable qualification which we should accommodate
(but of course with file-caching, that's not true, right?).

> > the permissions check is to ensure that we don't lose 
> > a race against someone renaming things). 
>  
> You seem to mean the stuff in OpenHiddenFile and IsOurFile.  Right? 

yes.
  
> So if I understand you right, stuff added in LYMain.c (subdir creation) 
> doesn't make the stuff in LYUtils.c any less necessary.  That's basically 

there's some overlap in coverage.  doesn't cover download/print operations,
which was iirc the original motivation for the logic in IsOurFile.

>   Klaus 

-- look on the bright side: the 2.8.3 release is not marked "FORBIDDEN"
   in FreeBSD any more.

-- 
Thomas E. Dickey  <address@hidden>
http://dickey.his.com
ftp://dickey.his.com

; To UNSUBSCRIBE: Send "unsubscribe lynx-dev" to address@hidden