This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
ODBM_File: Avoid TOCTOU and using negative returns.
authorJarkko Hietaniemi <jhi@iki.fi>
Wed, 3 Feb 2016 15:53:23 +0000 (10:53 -0500)
committerJarkko Hietaniemi <jhi@iki.fi>
Sun, 7 Feb 2016 13:23:46 +0000 (08:23 -0500)
Coverity CID 135022: Argument cannot be negative (NEGATIVE_RETURNS)
Coverity CID 135027: Time of check time of use (TOCTOU)

Replace use of stat()-guarded use of creat() (wow) with open(...O_EXCL...)
(when O_CREAT) so that there is no race condition (TOCTOU) window
between the stat() check for non-existence (which can fail also for
other reasons) and the two (sic) creat() calls.

Similarly, without O_CREAT, use open(...O_RDONLY...) instead of the stat().

Possible problem: arguably, systems old enough to be still using
ODBM_File (or requiring creat()) might not have the O_EXCL.

ext/ODBM_File/ODBM_File.xs

index 57beaf9..bf5def3 100644 (file)
@@ -92,7 +92,6 @@ odbm_TIEHASH(dbtype, filename, flags, mode)
        {
            char *tmpbuf;
            void * dbp ;
-            Stat_t statbuf;
            dMY_CXT;
 
            if (dbmrefcnt++)
@@ -100,16 +99,37 @@ odbm_TIEHASH(dbtype, filename, flags, mode)
            Newx(tmpbuf, strlen(filename) + 5, char);
            SAVEFREEPV(tmpbuf);
            sprintf(tmpbuf,"%s.dir",filename);
-            if (stat(tmpbuf, &statbuf) < 0) {
-               if (flags & O_CREAT) {
-                   if (mode < 0 || close(creat(tmpbuf,mode)) < 0)
-                       croak("ODBM_File: Can't create %s", filename);
-                   sprintf(tmpbuf,"%s.pag",filename);
-                   if (close(creat(tmpbuf,mode)) < 0)
-                       croak("ODBM_File: Can't create %s", filename);
-               }
-               else
-                   croak("ODBM_FILE: Can't open %s", filename);
+            if ((flags & O_CREAT)) {
+               const int oflags = O_CREAT | O_TRUNC | O_WRONLY | O_EXCL;
+               int created = 0;
+               int fd;
+               if (mode < 0)
+                   goto creat_done;
+               if ((fd = open(tmpbuf,oflags,mode)) < 0 && errno != EEXIST)
+                   goto creat_done;
+               if (close(fd) < 0)
+                   goto creat_done;
+               sprintf(tmpbuf,"%s.pag",filename);
+               if ((fd = open(tmpbuf,oflags,mode)) < 0 && errno != EEXIST)
+                   goto creat_done;
+               if (close(fd) < 0)
+                   goto creat_done;
+               created = 1;
+            creat_done:
+               if (!created)
+                   croak("ODBM_File: Can't create %s", filename);
+            }
+            else {
+               int opened = 0;
+               int fd;
+               if ((fd = open(tmpbuf,O_RDONLY,mode)) < 0)
+                   goto rdonly_done;
+               if (close(fd) < 0)
+                   goto rdonly_done;
+               opened = 1;
+            rdonly_done:
+               if (!opened)
+                   croak("ODBM_FILE: Can't open %s", filename);
            }
            dbp = (void*)(dbminit(filename) >= 0 ? &dbmrefcnt : 0);
            RETVAL = (ODBM_File)safecalloc(1, sizeof(ODBM_File_type));