Fix taint issue with retry records. Bug 2492
authorJeremy Harris <jgh146exb@wizmail.org>
Thu, 12 Dec 2019 23:43:10 +0000 (23:43 +0000)
committerJeremy Harris <jgh146exb@wizmail.org>
Thu, 12 Dec 2019 23:43:10 +0000 (23:43 +0000)
doc/doc-txt/ChangeLog
src/src/retry.c

index 1cc3d63c8d85a3dbb51984b796d487c20bfa4df4..f9a939d723cb2d56759149a37308a32356a682b3 100644 (file)
@@ -35,6 +35,10 @@ JH/08 Bug 2491: Use tainted buffers for the transport smtp context.  Previously
 JH/09 Bug 2493: Harden ARC verify against Outlook, whick has been seen to mix
       the ordering of its ARC headers.  This caused a crash.
 
 JH/09 Bug 2493: Harden ARC verify against Outlook, whick has been seen to mix
       the ordering of its ARC headers.  This caused a crash.
 
+JH/10 Bug 2492: Use tainted memory for retry record when needed.  Previously when
+      a new record was being constructed with information from the peer, a trap
+      was taken.
+
 
 Exim version 4.93
 -----------------
 
 Exim version 4.93
 -----------------
index d068f547d386f18beb1fee762ffc8a7a10cfcbde..175da216e6cf67842ee72169ab50705ee794400c 100644 (file)
@@ -659,7 +659,8 @@ for (int i = 0; i < 3; i++)
         /* Read a retry record from the database or construct a new one.
         Ignore an old one if it is too old since it was last updated. */
 
         /* Read a retry record from the database or construct a new one.
         Ignore an old one if it is too old since it was last updated. */
 
-        retry_record = dbfn_read(dbm_file, rti->key);
+        retry_record = dbfn_read_with_length(dbm_file, rti->key,
+                                             &message_space);
         if (  retry_record
           && now - retry_record->time_stamp > retry_data_expire)
           retry_record = NULL;
         if (  retry_record
           && now - retry_record->time_stamp > retry_data_expire)
           retry_record = NULL;
@@ -675,7 +676,7 @@ for (int i = 0; i < 3; i++)
           retry_record->expired = FALSE;
           retry_record->text[0] = 0;      /* just in case */
           }
           retry_record->expired = FALSE;
           retry_record->text[0] = 0;      /* just in case */
           }
-        else message_space = Ustrlen(retry_record->text);
+       else message_space -= sizeof(dbdata_retry);
 
         /* Compute how long this destination has been failing */
 
 
         /* Compute how long this destination has been failing */
 
@@ -806,15 +807,17 @@ for (int i = 0; i < 3; i++)
         if (next_try - now > retry_interval_max)
           next_try = now + retry_interval_max;
 
         if (next_try - now > retry_interval_max)
           next_try = now + retry_interval_max;
 
-        /* If the new message length is greater than the previous one, we
-        have to copy the record first. */
-
-        if (message_length > message_space)
-          {
-          dbdata_retry *newr = store_get(sizeof(dbdata_retry) + message_length, FALSE);
-          memcpy(newr, retry_record, sizeof(dbdata_retry));
-          retry_record = newr;
-          }
+        /* If the new message length is greater than the previous one, we have
+       to copy the record first.  If we're using an old one, the read used
+       tainted memory so we're ok to write into it. */
+
+       if (message_length > message_space)
+         {
+         dbdata_retry * newr =
+           store_get(sizeof(dbdata_retry) + message_length, is_tainted(message));
+         memcpy(newr, retry_record, sizeof(dbdata_retry));
+         retry_record = newr;
+         }
 
         /* Set up the retry record; message_length may be less than the string
         length for very long error strings. */
 
         /* Set up the retry record; message_length may be less than the string
         length for very long error strings. */