ODBM_File: Avoid TOCTOU and using negative returns.

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.

1 files changed, 31 insertions, 11 deletions

diff --git a/ext/ODBM_File/ODBM_File.xs b/ext/ODBM_File/ODBM_File.xs
index 57beaf9a93..bf5def38f8 100644
--- a/ext/ODBM_File/ODBM_File.xs
+++ b/ext/ODBM_File/ODBM_File.xs
@@ -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));