Harden plaintext authenticator

[users/heiko/exim.git] / src / src / dbfn.c

diff --git a/src/src/dbfn.c b/src/src/dbfn.c
index 928cf00f1c94becf68f07b006dd3da3ef09a75a2..336cfe73e262119c53f3322c73af77c336de9a03 100644 (file)
--- a/src/src/dbfn.c
+++ b/src/src/dbfn.c
@@ -1,10 +1,8 @@
-/* $Cambridge: exim/src/src/dbfn.c,v 1.3 2005/06/14 10:32:01 ph10 Exp $ */
-

 /*************************************************
 *     Exim - an Internet mail transport agent    *
 *************************************************/
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
 *************************************************/
 

-/* Copyright (c) University of Cambridge 1995 - 2005 */
+/* Copyright (c) University of Cambridge 1995 - 2018 */

 /* See the file NOTICE for conditions of use and distribution. */
 
 
 /* See the file NOTICE for conditions of use and distribution. */
 
 


@@ -80,6 +78,10 @@ Returns:   NULL if the open failed, or the locking failed. After locking
 
            On success, dbblock is returned. This contains the dbm pointer and
            the fd of the locked lock file.
 
            On success, dbblock is returned. This contains the dbm pointer and
            the fd of the locked lock file.

+
+There are some calls that use O_RDWR|O_CREAT for the flags. Having discovered
+this in December 2005, I'm not sure if this is correct or not, but for the
+moment I haven't changed them.

 */
 
 open_db *
 */
 
 open_db *


@@ -89,7 +91,9 @@ int rc, save_errno;
 BOOL read_only = flags == O_RDONLY;
 BOOL created = FALSE;
 flock_t lock_data;
 BOOL read_only = flags == O_RDONLY;
 BOOL created = FALSE;
 flock_t lock_data;

-uschar buffer[256];
+uschar dirname[256], filename[256];
+
+DEBUG(D_hints_lookup) acl_level++;

 
 /* The first thing to do is to open a separate file on which to lock. This
 ensures that Exim has exclusive use of the database before it even tries to
 
 /* The first thing to do is to open a separate file on which to lock. This
 ensures that Exim has exclusive use of the database before it even tries to


@@ -104,20 +108,22 @@ make the directory as well, just in case. We won't be doing this many times
 unnecessarily, because usually the lock file will be there. If the directory
 exists, there is no error. */
 
 unnecessarily, because usually the lock file will be there. If the directory
 exists, there is no error. */
 

-sprintf(CS buffer, "%s/db/%s.lockfile", spool_directory, name);
+snprintf(CS dirname, sizeof(dirname), "%s/db", spool_directory);
+snprintf(CS filename, sizeof(filename), "%s/%s.lockfile", dirname, name);

 
 

-if ((dbblock->lockfd = Uopen(buffer, O_RDWR, EXIMDB_LOCKFILE_MODE)) < 0)
+if ((dbblock->lockfd = Uopen(filename, O_RDWR, EXIMDB_LOCKFILE_MODE)) < 0)

   {
   created = TRUE;
   (void)directory_make(spool_directory, US"db", EXIMDB_DIRECTORY_MODE, TRUE);
   {
   created = TRUE;
   (void)directory_make(spool_directory, US"db", EXIMDB_DIRECTORY_MODE, TRUE);

-  dbblock->lockfd = Uopen(buffer, O_RDWR|O_CREAT, EXIMDB_LOCKFILE_MODE);
+  dbblock->lockfd = Uopen(filename, O_RDWR|O_CREAT, EXIMDB_LOCKFILE_MODE);

   }
 
 if (dbblock->lockfd < 0)
   {
   log_write(0, LOG_MAIN, "%s",
   }
 
 if (dbblock->lockfd < 0)
   {
   log_write(0, LOG_MAIN, "%s",

-    string_open_failed(errno, "database lock file %s", buffer));
+    string_open_failed(errno, "database lock file %s", filename));

   errno = 0;      /* Indicates locking failure */
   errno = 0;      /* Indicates locking failure */

+  DEBUG(D_hints_lookup) acl_level--;

   return NULL;
   }
 
   return NULL;
   }
 


@@ -128,41 +134,45 @@ lock_data.l_type = read_only? F_RDLCK : F_WRLCK;
 lock_data.l_whence = lock_data.l_start = lock_data.l_len = 0;
 
 DEBUG(D_hints_lookup|D_retry|D_route|D_deliver)
 lock_data.l_whence = lock_data.l_start = lock_data.l_len = 0;
 
 DEBUG(D_hints_lookup|D_retry|D_route|D_deliver)

-  debug_printf("locking %s\n", buffer);
+  debug_printf_indent("locking %s\n", filename);

 
 sigalrm_seen = FALSE;
 
 sigalrm_seen = FALSE;

-alarm(EXIMDB_LOCK_TIMEOUT);
+ALARM(EXIMDB_LOCK_TIMEOUT);

 rc = fcntl(dbblock->lockfd, F_SETLKW, &lock_data);
 rc = fcntl(dbblock->lockfd, F_SETLKW, &lock_data);

-alarm(0);
+ALARM_CLR(0);

 
 if (sigalrm_seen) errno = ETIMEDOUT;
 if (rc < 0)
   {
 
 if (sigalrm_seen) errno = ETIMEDOUT;
 if (rc < 0)
   {

-  log_write(0, LOG_MAIN, "Failed to get %s lock for %s: %s",
-    ((flags & O_RDONLY) != 0)? "read" : "write", buffer,
-    (errno == ETIMEDOUT)? "timed out" : strerror(errno));
-  close(dbblock->lockfd);
+  log_write(0, LOG_MAIN|LOG_PANIC, "Failed to get %s lock for %s: %s",
+    read_only ? "read" : "write", filename,
+    errno == ETIMEDOUT ? "timed out" : strerror(errno));
+  (void)close(dbblock->lockfd);

   errno = 0;       /* Indicates locking failure */
   errno = 0;       /* Indicates locking failure */

+  DEBUG(D_hints_lookup) acl_level--;

   return NULL;
   }
 
   return NULL;
   }
 

-DEBUG(D_hints_lookup) debug_printf("locked %s\n", buffer);
+DEBUG(D_hints_lookup) debug_printf_indent("locked  %s\n", filename);

 
 /* At this point we have an opened and locked separate lock file, that is,
 exclusive access to the database, so we can go ahead and open it. If we are
 expected to create it, don't do so at first, again so that we can detect
 whether we need to change its ownership (see comments about the lock file
 
 /* At this point we have an opened and locked separate lock file, that is,
 exclusive access to the database, so we can go ahead and open it. If we are
 expected to create it, don't do so at first, again so that we can detect
 whether we need to change its ownership (see comments about the lock file

-above.) */
+above.) There have been regular reports of crashes while opening hints
+databases - often this is caused by non-matching db.h and the library. To make
+it easy to pin this down, there are now debug statements on either side of the
+open call. */

 
 

-sprintf(CS buffer, "%s/db/%s", spool_directory, name);
-EXIM_DBOPEN(buffer, flags, EXIMDB_MODE, &(dbblock->dbptr));
+snprintf(CS filename, sizeof(filename), "%s/%s", dirname, name);
+EXIM_DBOPEN(filename, dirname, flags, EXIMDB_MODE, &(dbblock->dbptr));

 
 

-if (dbblock->dbptr == NULL && errno == ENOENT && flags == O_RDWR)
+if (!dbblock->dbptr && errno == ENOENT && flags == O_RDWR)

   {
   DEBUG(D_hints_lookup)
   {
   DEBUG(D_hints_lookup)

-    debug_printf("%s appears not to exist: trying to create\n", buffer);
+    debug_printf_indent("%s appears not to exist: trying to create\n", filename);

   created = TRUE;
   created = TRUE;

-  EXIM_DBOPEN(buffer, flags|O_CREAT, EXIMDB_MODE, &(dbblock->dbptr));
+  EXIM_DBOPEN(filename, dirname, flags|O_CREAT, EXIMDB_MODE, &(dbblock->dbptr));

   }
 
 save_errno = errno;
   }
 
 save_errno = errno;


@@ -185,51 +195,53 @@ if (created && geteuid() == root_uid)
   {
   DIR *dd;
   struct dirent *ent;
   {
   DIR *dd;
   struct dirent *ent;

-  uschar *lastname = Ustrrchr(buffer, '/') + 1;
+  uschar *lastname = Ustrrchr(filename, '/') + 1;

   int namelen = Ustrlen(name);
 
   *lastname = 0;
   int namelen = Ustrlen(name);
 
   *lastname = 0;

-  dd = opendir(CS buffer);
+  dd = opendir(CS filename);

 
 

-  while ((ent = readdir(dd)) != NULL)
-    {
+  while ((ent = readdir(dd)))

     if (Ustrncmp(ent->d_name, name, namelen) == 0)
       {
       struct stat statbuf;
       Ustrcpy(lastname, ent->d_name);
     if (Ustrncmp(ent->d_name, name, namelen) == 0)
       {
       struct stat statbuf;
       Ustrcpy(lastname, ent->d_name);

-      if (Ustat(buffer, &statbuf) >= 0 && statbuf.st_uid != exim_uid)
+      if (Ustat(filename, &statbuf) >= 0 && statbuf.st_uid != exim_uid)

         {
         {

-        DEBUG(D_hints_lookup) debug_printf("ensuring %s is owned by exim\n", buffer);
-        Uchown(buffer, exim_uid, exim_gid);
+        DEBUG(D_hints_lookup) debug_printf_indent("ensuring %s is owned by exim\n", filename);
+        if (Uchown(filename, exim_uid, exim_gid))
+          DEBUG(D_hints_lookup) debug_printf_indent("failed setting %s to owned by exim\n", filename);

         }
       }
         }
       }

-    }

 
   closedir(dd);
   }
 
 /* If the open has failed, return NULL, leaving errno set. If lof is TRUE,
 
   closedir(dd);
   }
 
 /* If the open has failed, return NULL, leaving errno set. If lof is TRUE,

-log the event - also for debugging - but not if the file just doesn't exist. */
+log the event - also for debugging - but debug only if the file just doesn't
+exist. */

 
 

-if (dbblock->dbptr == NULL)
+if (!dbblock->dbptr)

   {
   {

-  if (save_errno != ENOENT)
-    {
-    if (lof)
-      log_write(0, LOG_MAIN, "%s", string_open_failed(save_errno, "DB file %s",
-        buffer));
-    else
-      DEBUG(D_hints_lookup)
-        debug_printf("%s", CS string_open_failed(save_errno, "DB file %s\n",
-          buffer));
-    }
-  close(dbblock->lockfd);
+  if (lof && save_errno != ENOENT)
+    log_write(0, LOG_MAIN, "%s", string_open_failed(save_errno, "DB file %s",
+        filename));
+  else
+    DEBUG(D_hints_lookup)
+      debug_printf_indent("%s\n", CS string_open_failed(save_errno, "DB file %s",
+          filename));
+  (void)close(dbblock->lockfd);

   errno = save_errno;
   errno = save_errno;

+  DEBUG(D_hints_lookup) acl_level--;

   return NULL;
   }
 
 DEBUG(D_hints_lookup)
   return NULL;
   }
 
 DEBUG(D_hints_lookup)

-  debug_printf("opened hints database %s: flags=%x\n", buffer, flags);
+  debug_printf_indent("opened hints database %s: flags=%s\n", filename,
+    flags == O_RDONLY ? "O_RDONLY"
+    : flags == O_RDWR ? "O_RDWR"
+    : flags == (O_RDWR|O_CREAT) ? "O_RDWR|O_CREAT"
+    : "??");

 
 /* Pass back the block containing the opened database handle and the open fd
 for the lock. */
 
 /* Pass back the block containing the opened database handle and the open fd
 for the lock. */


@@ -255,7 +267,9 @@ void
 dbfn_close(open_db *dbblock)
 {
 EXIM_DBCLOSE(dbblock->dbptr);
 dbfn_close(open_db *dbblock)
 {
 EXIM_DBCLOSE(dbblock->dbptr);

-close(dbblock->lockfd);
+(void)close(dbblock->lockfd);
+DEBUG(D_hints_lookup)
+  { debug_printf_indent("closed hints database and lockfile\n"); acl_level--; }

 }
 
 
 }
 
 


@@ -283,17 +297,21 @@ Returns: a pointer to the retrieved record, or
 */
 
 void *
 */
 
 void *

-dbfn_read_with_length(open_db *dbblock, uschar *key, int *length)
+dbfn_read_with_length(open_db *dbblock, const uschar *key, int *length)

 {
 void *yield;
 EXIM_DATUM key_datum, result_datum;
 {
 void *yield;
 EXIM_DATUM key_datum, result_datum;

+int klen = Ustrlen(key) + 1;
+uschar * key_copy = store_get(klen);

 
 

-DEBUG(D_hints_lookup) debug_printf("dbfn_read: key=%s\n", key);
+memcpy(key_copy, key, klen);
+
+DEBUG(D_hints_lookup) debug_printf_indent("dbfn_read: key=%s\n", key);

 
 EXIM_DATUM_INIT(key_datum);         /* Some DBM libraries require the datum */
 EXIM_DATUM_INIT(result_datum);      /* to be cleared before use. */
 
 EXIM_DATUM_INIT(key_datum);         /* Some DBM libraries require the datum */
 EXIM_DATUM_INIT(result_datum);      /* to be cleared before use. */

-EXIM_DATUM_DATA(key_datum) = CS key;
-EXIM_DATUM_SIZE(key_datum) = Ustrlen(key) + 1;
+EXIM_DATUM_DATA(key_datum) = CS key_copy;
+EXIM_DATUM_SIZE(key_datum) = klen;

 
 if (!EXIM_DBGET(dbblock->dbptr, key_datum, result_datum)) return NULL;
 
 
 if (!EXIM_DBGET(dbblock->dbptr, key_datum, result_datum)) return NULL;
 


@@ -323,18 +341,22 @@ Returns:    the yield of the underlying dbm or db "write" function. If this
 */
 
 int
 */
 
 int

-dbfn_write(open_db *dbblock, uschar *key, void *ptr, int length)
+dbfn_write(open_db *dbblock, const uschar *key, void *ptr, int length)

 {
 EXIM_DATUM key_datum, value_datum;
 dbdata_generic *gptr = (dbdata_generic *)ptr;
 {
 EXIM_DATUM key_datum, value_datum;
 dbdata_generic *gptr = (dbdata_generic *)ptr;

+int klen = Ustrlen(key) + 1;
+uschar * key_copy = store_get(klen);
+
+memcpy(key_copy, key, klen);

 gptr->time_stamp = time(NULL);
 
 gptr->time_stamp = time(NULL);
 

-DEBUG(D_hints_lookup) debug_printf("dbfn_write: key=%s\n", key);
+DEBUG(D_hints_lookup) debug_printf_indent("dbfn_write: key=%s\n", key);

 
 EXIM_DATUM_INIT(key_datum);         /* Some DBM libraries require the datum */
 EXIM_DATUM_INIT(value_datum);       /* to be cleared before use. */
 
 EXIM_DATUM_INIT(key_datum);         /* Some DBM libraries require the datum */
 EXIM_DATUM_INIT(value_datum);       /* to be cleared before use. */

-EXIM_DATUM_DATA(key_datum) = CS key;
-EXIM_DATUM_SIZE(key_datum) = Ustrlen(key) + 1;
+EXIM_DATUM_DATA(key_datum) = CS key_copy;
+EXIM_DATUM_SIZE(key_datum) = klen;

 EXIM_DATUM_DATA(value_datum) = CS ptr;
 EXIM_DATUM_SIZE(value_datum) = length;
 return EXIM_DBPUT(dbblock->dbptr, key_datum, value_datum);
 EXIM_DATUM_DATA(value_datum) = CS ptr;
 EXIM_DATUM_SIZE(value_datum) = length;
 return EXIM_DBPUT(dbblock->dbptr, key_datum, value_datum);


@@ -355,12 +377,18 @@ Returns: the yield of the underlying dbm or db "delete" function.
 */
 
 int
 */
 
 int

-dbfn_delete(open_db *dbblock, uschar *key)
+dbfn_delete(open_db *dbblock, const uschar *key)

 {
 {

+int klen = Ustrlen(key) + 1;
+uschar * key_copy = store_get(klen);
+
+DEBUG(D_hints_lookup) debug_printf_indent("dbfn_delete: key=%s\n", key);
+
+memcpy(key_copy, key, klen);

 EXIM_DATUM key_datum;
 EXIM_DATUM_INIT(key_datum);         /* Some DBM libraries require clearing */
 EXIM_DATUM key_datum;
 EXIM_DATUM_INIT(key_datum);         /* Some DBM libraries require clearing */

-EXIM_DATUM_DATA(key_datum) = CS key;
-EXIM_DATUM_SIZE(key_datum) = Ustrlen(key) + 1;
+EXIM_DATUM_DATA(key_datum) = CS key_copy;
+EXIM_DATUM_SIZE(key_datum) = klen;

 return EXIM_DBDEL(dbblock->dbptr, key_datum);
 }
 
 return EXIM_DBDEL(dbblock->dbptr, key_datum);
 }
 


@@ -389,6 +417,8 @@ EXIM_DATUM key_datum, value_datum;
 uschar *yield;
 value_datum = value_datum;    /* dummy; not all db libraries use this */
 
 uschar *yield;
 value_datum = value_datum;    /* dummy; not all db libraries use this */
 

+DEBUG(D_hints_lookup) debug_printf_indent("dbfn_scan\n");
+

 /* Some dbm require an initialization */
 
 if (start) EXIM_DBCREATE_CURSOR(dbblock->dbptr, cursor);
 /* Some dbm require an initialization */
 
 if (start) EXIM_DBCREATE_CURSOR(dbblock->dbptr, cursor);


@@ -509,7 +539,7 @@ while (Ufgets(buffer, 256, stdin) != NULL)
     odb = dbfn_open(s, O_RDWR, dbblock + i, TRUE);
     stop = clock();
 
     odb = dbfn_open(s, O_RDWR, dbblock + i, TRUE);
     stop = clock();
 

-    if (odb != NULL)
+    if (odb)

       {
       current = i;
       printf("opened %d\n", current);
       {
       current = i;
       printf("opened %d\n", current);