develooper Front page | perl.perl5.porters | Postings from December 2008

Re: [perl #60904] [PATCH] Race condition with perl -i.bk

Thread Previous | Thread Next
From:
Chip Salzenberg
Date:
December 12, 2008 18:38
Subject:
Re: [perl #60904] [PATCH] Race condition with perl -i.bk
Message ID:
20081213023812.GE6192@tytlal.topaz.cx
On Mon, Dec 01, 2008 at 03:01:12PM -0800, Chip Salzenberg wrote:
> On Fri, Nov 28, 2008 at 09:18:31AM -0800, skasal@redhat.com (via RT) wrote:
> > when I call "perl -pi.bk -e 's/x/y/' foo, thge strace log contains this:
> >   rename("foo", "foo.bk") = 0
> >   open("foo", ..., 0666)  = 4
> >   ...
> >   fchmod(4, 0100600)      = 0
> > This means that the new file is created using the final filename "foo"
> > (which is predictable) and with mode 666 (modulo umask), which means
> > the file is group or even world writable.
> > 
> > I'd suggest changing that mode to "0600" or "0400".
> 
> It's a fair cop, but society's to blame.
> Here's a patch, which not only fixes the bug by using mode 0600, but also
> removes tests of PL_inplace in places where it has already been tested.

The patch below was not applied.  Is something wrong with it, or did it
perhaps just get overlooked?

diff --git a/doio.c b/doio.c
index ce57ae1..9dc28b3 100644
--- a/doio.c
+++ b/doio.c
@@ -811,8 +811,7 @@ Perl_nextargv(pTHX_ register GV *gv)
 		    do_close(gv,FALSE);
 		    (void)PerlLIO_unlink(SvPVX_const(sv));
 		    (void)PerlLIO_rename(PL_oldname,SvPVX_const(sv));
-		    do_open(gv,(char*)SvPVX_const(sv),SvCUR(sv),PL_inplace!=0,
-			    O_RDONLY,0,NULL);
+		    do_open(gv,(char*)SvPVX_const(sv),SvCUR(sv),TRUE,O_RDONLY,0,NULL);
 #endif /* DOSISH */
 #else
 		    (void)UNLINK(SvPVX_const(sv));
@@ -844,17 +843,15 @@ Perl_nextargv(pTHX_ register GV *gv)
 #endif
 		}
 
-		sv_setpvn(sv,">",!PL_inplace);
-		sv_catpvn(sv,PL_oldname,oldlen);
+		sv_setpvn(sv,PL_oldname,oldlen);
 		SETERRNO(0,0);		/* in case sprintf set errno */
+		if (!do_open(PL_argvoutgv,(char*)SvPVX_const(sv),SvCUR(sv),TRUE,
 #ifdef VMS
-		if (!do_open(PL_argvoutgv,(char*)SvPVX_const(sv),SvCUR(sv),
-			     PL_inplace!=0,O_WRONLY|O_CREAT|O_TRUNC,0,NULL))
+			     O_WRONLY|O_CREAT|O_TRUNC,0,
 #else
-		    if (!do_open(PL_argvoutgv,(char*)SvPVX_const(sv),SvCUR(sv),
-			     PL_inplace!=0,O_WRONLY|O_CREAT|OPEN_EXCL,0666,
-			     NULL))
+			     O_WRONLY|O_CREAT|OPEN_EXCL,0600,
 #endif
+			     NULL))
 		{
 		    if (ckWARN_d(WARN_INPLACE))	
 		        Perl_warner(aTHX_ packWARN(WARN_INPLACE), "Can't do inplace edit on %s: %s",

-- 
Chip Salzenberg   twitter:chipsalz
"UTOPIA PLANITIA IS IN ORBIT is the new HAN SHOT FIRST" - Crisper Than Thou

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About