Fix taint trap in parse_fix_phrase(). Bug 2617

(cherry picked from commit 3c90bbcdc7cf73298156f7bcd5f5e750e7814e72)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 08c6638a9292dc41b15120e242bdeee60357a17b..a1f39459e728252b4e3a8af5d24c092211d23aa9 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -72,6 +72,12 @@ JH/17 Bug 2295: Fix DKIM signing to always semicolon-terminate.  Although the
       intended but triggered by a line-wrap alignement.  Discovery and fix by
       Guillaume Outters, hacked on by JH.
 
       intended but triggered by a line-wrap alignement.  Discovery and fix by
       Guillaume Outters, hacked on by JH.
 

+JH/18 Bug 2617: Fix a taint trap in parse_fix_phrase().  Previously when the
+      name being quoted was tainted a trap would be taken.  Fix by using
+      dynamicaly created buffers.  The routine could have been called by a
+      rewrite with the "h" flag, by using the "-F" command-line option, or
+      by using a "name=" option on a control=submission ACL modifier.
+

 
 Exim version 4.94
 -----------------
 
 Exim version 4.94
 -----------------

diff --git a/src/src/acl.c b/src/src/acl.c
index 62cb6856179e7868c85014e93e35bd3ebbd160f0..105b1b473999b3cae45a9756d058c74a6f3e89cc 100644 (file)
--- a/src/src/acl.c
+++ b/src/src/acl.c
@@ -3199,8 +3199,7 @@ for (; cb; cb = cb->next)
              {
              const uschar *pp = p + 6;
              while (*pp) pp++;
              {
              const uschar *pp = p + 6;
              while (*pp) pp++;

-             submission_name = string_copy(parse_fix_phrase(p+6, pp-p-6,
-               big_buffer, big_buffer_size));
+             submission_name = parse_fix_phrase(p+6, pp-p-6);

              p = pp;
              }
            else break;
              p = pp;
              }
            else break;

diff --git a/src/src/exim.c b/src/src/exim.c
index dde99106501a25e9dd07da954b7c9860ec05cdd7..ac0ff552317bc14a7bb5c88962737b6509e19733 100644 (file)
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -4769,8 +4769,7 @@ if (originator_login == NULL || f.running_in_test_harness)
 /* Ensure that the user name is in a suitable form for use as a "phrase" in an
 RFC822 address.*/
 
 /* Ensure that the user name is in a suitable form for use as a "phrase" in an
 RFC822 address.*/
 

-originator_name = string_copy(parse_fix_phrase(originator_name,
-  Ustrlen(originator_name), big_buffer, big_buffer_size));
+originator_name = parse_fix_phrase(originator_name, Ustrlen(originator_name));

 
 /* If a message is created by this call of Exim, the uid/gid of its originator
 are those of the caller. These values are overridden if an existing message is
 
 /* If a message is created by this call of Exim, the uid/gid of its originator
 are those of the caller. These values are overridden if an existing message is

diff --git a/src/src/expand.c b/src/src/expand.c
index 6ed22c14d958a95a80a481f3264b4fd39fbccc4c..791222324b6f261d35aeef8dc9ef47d0f6673b37 100644 (file)
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -7571,13 +7571,10 @@ while (*s != 0)
       prescribed by the RFC, if there are characters that need to be encoded */
 
       case EOP_RFC2047:
       prescribed by the RFC, if there are characters that need to be encoded */
 
       case EOP_RFC2047:

-        {
-        uschar buffer[2048];

         yield = string_cat(yield,
                            parse_quote_2047(sub, Ustrlen(sub), headers_charset,
         yield = string_cat(yield,
                            parse_quote_2047(sub, Ustrlen(sub), headers_charset,

-                             buffer, sizeof(buffer), FALSE));
+                             FALSE));

         continue;
         continue;

-        }

 
       /* RFC 2047 decode */
 
 
       /* RFC 2047 decode */
 

diff --git a/src/src/functions.h b/src/src/functions.h
index afe9f1bf4883d05c7b7b1c30c20ec109418f2a21..f4d1622dc205fbee34d042e19d08c4a08e9acdd8 100644 (file)
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -366,9 +366,9 @@ extern int     parse_forward_list(uschar *, int, address_item **, uschar **,
                  const uschar *, uschar *, error_block **);
 extern uschar *parse_find_address_end(uschar *, BOOL);
 extern uschar *parse_find_at(uschar *);
                  const uschar *, uschar *, error_block **);
 extern uschar *parse_find_address_end(uschar *, BOOL);
 extern uschar *parse_find_at(uschar *);

-extern const uschar *parse_fix_phrase(const uschar *, int, uschar *, int);
+extern const uschar *parse_fix_phrase(const uschar *, int);

 extern uschar *parse_message_id(uschar *, uschar **, uschar **);
 extern uschar *parse_message_id(uschar *, uschar **, uschar **);

-extern const uschar *parse_quote_2047(const uschar *, int, uschar *, uschar *, int, BOOL);
+extern const uschar *parse_quote_2047(const uschar *, int, uschar *, BOOL);

 extern uschar *parse_date_time(uschar *str, time_t *t);
 extern int     vaguely_random_number(int);
 #ifndef DISABLE_TLS
 extern uschar *parse_date_time(uschar *str, time_t *t);
 extern int     vaguely_random_number(int);
 #ifndef DISABLE_TLS

diff --git a/src/src/parse.c b/src/src/parse.c
index e3b471f1a6e7a017171e00f4668ebc1c9a1e13d6..3ea758ac9a0b18e23d5937268c5554c1ce3dee38 100644 (file)
--- a/src/src/parse.c
+++ b/src/src/parse.c
@@ -843,8 +843,7 @@ return NULL;
 
 /* This function is used for quoting text in headers according to RFC 2047.
 If the only characters that strictly need quoting are spaces, we return the
 
 /* This function is used for quoting text in headers according to RFC 2047.
 If the only characters that strictly need quoting are spaces, we return the

-original string, unmodified. If a quoted string is too long for the buffer, it
-is truncated. (This shouldn't happen: this is normally handling short strings.)
+original string, unmodified.

 
 Hmmph. As always, things get perverted for other uses. This function was
 originally for the "phrase" part of addresses. Now it is being used for much
 
 Hmmph. As always, things get perverted for other uses. This function was
 originally for the "phrase" part of addresses. Now it is being used for much


@@ -856,77 +855,57 @@ Arguments:
                  chars
   len          the length of the string
   charset      the name of the character set; NULL => iso-8859-1
                  chars
   len          the length of the string
   charset      the name of the character set; NULL => iso-8859-1

-  buffer       the buffer to put the answer in
-  buffer_size  the size of the buffer

   fold         if TRUE, a newline is inserted before the separating space when
                  more than one encoded-word is generated
 
 Returns:       pointer to the original string, if no quoting needed, or
   fold         if TRUE, a newline is inserted before the separating space when
                  more than one encoded-word is generated
 
 Returns:       pointer to the original string, if no quoting needed, or

-               pointer to buffer containing the quoted string, or
-               a pointer to "String too long" if the buffer can't even hold
-               the introduction
+               pointer to allocated memory containing the quoted string

 */
 
 const uschar *
 */
 
 const uschar *

-parse_quote_2047(const uschar *string, int len, uschar *charset, uschar *buffer,
-  int buffer_size, BOOL fold)
+parse_quote_2047(const uschar *string, int len, uschar *charset, BOOL fold)

 {
 {

-const uschar *s = string;
-uschar *p, *t;
-int hlen;
+const uschar * s = string;
+int hlen, l;

 BOOL coded = FALSE;
 BOOL first_byte = FALSE;
 BOOL coded = FALSE;
 BOOL first_byte = FALSE;

+gstring * g =
+  string_fmt_append(NULL, "=?%s?Q?", charset ? charset : US"iso-8859-1");

 
 

-if (!charset) charset = US"iso-8859-1";
+hlen = l = g->ptr;

 
 

-/* We don't expect this to fail! */
-
-if (!string_format(buffer, buffer_size, "=?%s?Q?", charset))
-  return US"String too long";
-
-hlen = Ustrlen(buffer);
-t = buffer + hlen;
-p = buffer;
-
-for (; len > 0; len--)
+for (s = string; len > 0; s++, len--)

   {
   {

-  int ch = *s++;
-  if (t > buffer + buffer_size - hlen - 8) break;
+  int ch = *s;

 
 

-  if ((t - p > 67) && !first_byte)
+  if (g->ptr - l > 67 && !first_byte)

     {
     {

-    *t++ = '?';
-    *t++ = '=';
-    if (fold) *t++ = '\n';
-    *t++ = ' ';
-    p = t;
-    Ustrncpy(p, buffer, hlen);
-    t += hlen;
+    g = fold ? string_catn(g, US"?=\n ", 4) : string_catn(g, US"?= ", 3);
+    l = g->ptr;
+    g = string_catn(g, g->s, hlen);

     }
 
     }
 

-  if (ch < 33 || ch > 126 ||
-      Ustrchr("?=()<>@,;:\\\".[]_", ch) != NULL)
+  if (  ch < 33 || ch > 126
+     || Ustrchr("?=()<>@,;:\\\".[]_", ch) != NULL)

     {
     if (ch == ' ')
       {
     {
     if (ch == ' ')
       {

-      *t++ = '_';
+      g = string_catn(g, US"_", 1);

       first_byte = FALSE;
       }
     else
       {
       first_byte = FALSE;
       }
     else
       {

-      t += sprintf(CS t, "=%02X", ch);
+      g = string_fmt_append(g, "=%02X", ch);

       coded = TRUE;
       first_byte = !first_byte;
       }
     }
       coded = TRUE;
       first_byte = !first_byte;
       }
     }

-  else { *t++ = ch; first_byte = FALSE; }
+  else
+    { g = string_catn(g, s, 1); first_byte = FALSE; }

   }
 
   }
 

-*t++ = '?';
-*t++ = '=';
-*t = 0;
-
-return coded ? buffer : string;
+g = string_catn(g, US"?=", 2);
+return coded ? string_from_gstring(g) : string;

 }
 
 
 }
 
 


@@ -969,32 +948,25 @@ August 2000: Additional code added:
   We *could* use this for all cases, getting rid of the messy original code,
   but leave it for now. It would complicate simple cases like "John Q. Smith".
 
   We *could* use this for all cases, getting rid of the messy original code,
   but leave it for now. It would complicate simple cases like "John Q. Smith".
 

-The result is passed back in the buffer; it is usually going to be added to
-some other string. In order to be sure there is going to be no overflow,
-restrict the length of the input to 1/4 of the buffer size - this allows for
-every single character to be quoted or encoded without overflowing, and that
-wouldn't happen because of amalgamation. If the phrase is too long, return a
-fixed string.
+The result is passed back in allocated memory.

 
 Arguments:
   phrase       an RFC822 phrase
   len          the length of the phrase
 
 Arguments:
   phrase       an RFC822 phrase
   len          the length of the phrase

-  buffer       a buffer to put the result in
-  buffer_size  the size of the buffer

 
 Returns:       the fixed RFC822 phrase
 */
 
 const uschar *
 
 Returns:       the fixed RFC822 phrase
 */
 
 const uschar *

-parse_fix_phrase(const uschar *phrase, int len, uschar *buffer, int buffer_size)
+parse_fix_phrase(const uschar *phrase, int len)

 {
 int ch, i;
 BOOL quoted = FALSE;
 const uschar *s, *end;
 {
 int ch, i;
 BOOL quoted = FALSE;
 const uschar *s, *end;

+uschar * buffer;

 uschar *t, *yield;
 
 while (len > 0 && isspace(*phrase)) { phrase++; len--; }
 uschar *t, *yield;
 
 while (len > 0 && isspace(*phrase)) { phrase++; len--; }

-if (len > buffer_size/4) return US"Name too long";

 
 /* See if there are any non-printing characters, and if so, use the RFC 2047
 encoding for the whole thing. */
 
 /* See if there are any non-printing characters, and if so, use the RFC 2047
 encoding for the whole thing. */


@@ -1002,11 +974,13 @@ encoding for the whole thing. */
 for (i = 0, s = phrase; i < len; i++, s++)
   if ((*s < 32 && *s != '\t') || *s > 126) break;
 
 for (i = 0, s = phrase; i < len; i++, s++)
   if ((*s < 32 && *s != '\t') || *s > 126) break;
 

-if (i < len) return parse_quote_2047(phrase, len, headers_charset, buffer,
-  buffer_size, FALSE);
+if (i < len)
+  return parse_quote_2047(phrase, len, headers_charset, FALSE);

 
 /* No non-printers; use the RFC 822 quoting rules */
 
 
 /* No non-printers; use the RFC 822 quoting rules */
 

+buffer = store_get(len*4, is_tainted(phrase));
+

 s = phrase;
 end = s + len;
 yield = t = buffer + 1;
 s = phrase;
 end = s + len;
 yield = t = buffer + 1;


@@ -1173,6 +1147,7 @@ while (s < end)
   }
 
 *t = 0;
   }
 
 *t = 0;

+store_release_above(t+1);

 return yield;
 }
 
 return yield;
 }
 


@@ -2102,7 +2077,6 @@ int main(void)
 {
 int start, end, domain;
 uschar buffer[1024];
 {
 int start, end, domain;
 uschar buffer[1024];

-uschar outbuff[1024];

 
 big_buffer = store_malloc(big_buffer_size);
 
 
 big_buffer = store_malloc(big_buffer_size);
 


@@ -2115,8 +2089,7 @@ while (Ufgets(buffer, sizeof(buffer), stdin) != NULL)
   {
   buffer[Ustrlen(buffer)-1] = 0;
   if (buffer[0] == 0) break;
   {
   buffer[Ustrlen(buffer)-1] = 0;
   if (buffer[0] == 0) break;

-  printf("%s\n", CS parse_fix_phrase(buffer, Ustrlen(buffer), outbuff,
-    sizeof(outbuff)));
+  printf("%s\n", CS parse_fix_phrase(buffer, Ustrlen(buffer)));

   }
 
 printf("Testing parse_extract_address without group syntax and without UTF-8\n");
   }
 
 printf("Testing parse_extract_address without group syntax and without UTF-8\n");

diff --git a/src/src/rewrite.c b/src/src/rewrite.c
index f942bec051a841cca46973b1a49415ab9fd05507..7bff8a27318bd9959ab8e650e5c395bf368ccd6d 100644 (file)
--- a/src/src/rewrite.c
+++ b/src/src/rewrite.c
@@ -292,16 +292,11 @@ for (rewrite_rule * rule = rewrite_rules;
         uschar *p1 = new + start - 1;
         uschar *p2 = new + end + 1;
         const uschar *pf1, *pf2;
         uschar *p1 = new + start - 1;
         uschar *p2 = new + end + 1;
         const uschar *pf1, *pf2;

-        uschar buff1[256], buff2[256];

 
         while (p1 > new && p1[-1] == ' ') p1--;
 
         while (p1 > new && p1[-1] == ' ') p1--;

-        pf1 = parse_fix_phrase(new, p1 - new, buff1, sizeof(buff1));
+        pf1 = parse_fix_phrase(new, p1 - new);

         while (*p2 == ' ') p2++;
         while (*p2 == ' ') p2++;

-        pf2 = parse_fix_phrase(p2, Ustrlen(p2), buff2, sizeof(buff2));
-
-        /* Note that pf1 and pf2 are NOT necessarily buff1 and buff2. For
-        a non-RFC 2047 phrase that does not need to be RFC 2822 quoted, they
-        will be buff1+1 and buff2+1. */
+        pf2 = parse_fix_phrase(p2, Ustrlen(p2));

 
         start = Ustrlen(pf1) + start + new - p1;
         end = start + Ustrlen(newparsed);
 
         start = Ustrlen(pf1) + start + new - p1;
         end = start + Ustrlen(newparsed);

diff --git a/src/src/sieve.c b/src/src/sieve.c
index 18aa3e609ead7f995d2bcde2bbd63473d0379e79..2038f336b7b1e3b240f45d9bceae35eb3593a7cc 100644 (file)
--- a/src/src/sieve.c
+++ b/src/src/sieve.c
@@ -3087,11 +3087,8 @@ while (*filter->pc)
             if ((pid = child_open_exim2(&fd, envelope_from, envelope_from,
                        US"sieve-notify")) >= 1)
               {
             if ((pid = child_open_exim2(&fd, envelope_from, envelope_from,
                        US"sieve-notify")) >= 1)
               {

-              FILE *f;
-              uschar *buffer;
-              int buffer_capacity;
+              FILE * f = fdopen(fd, "wb");

 
 

-              f = fdopen(fd, "wb");

               fprintf(f,"From: %s\n", from.length == -1
                ? expand_string(US"$local_part_prefix$local_part$local_part_suffix@$domain")
                : from.character);
               fprintf(f,"From: %s\n", from.length == -1
                ? expand_string(US"$local_part_prefix$local_part$local_part_suffix@$domain")
                : from.character);


@@ -3104,12 +3101,9 @@ while (*filter->pc)
                 message.character=US"Notification";
                 message.length=Ustrlen(message.character);
                 }
                 message.character=US"Notification";
                 message.length=Ustrlen(message.character);
                 }

-              /* Allocation is larger than necessary, but enough even for split MIME words */
-              buffer_capacity = 32 + 4*message.length;
-              buffer=store_get(buffer_capacity, TRUE);

               if (message.length != -1)
                fprintf(f, "Subject: %s\n", parse_quote_2047(message.character,
               if (message.length != -1)
                fprintf(f, "Subject: %s\n", parse_quote_2047(message.character,

-                 message.length, US"utf-8", buffer, buffer_capacity, TRUE));
+                 message.length, US"utf-8", TRUE));

               fprintf(f,"\n");
               if (body.length>0) fprintf(f,"%s\n",body.character);
               fflush(f);
               fprintf(f,"\n");
               if (body.length>0) fprintf(f,"%s\n",body.character);
               fflush(f);


@@ -3263,8 +3257,6 @@ while (*filter->pc)
     if (exec)
       {
       address_item *addr;
     if (exec)
       {
       address_item *addr;

-      uschar *buffer;
-      int buffer_capacity;

       md5 base;
       uschar digest[16];
       uschar hexdigest[33];
       md5 base;
       uschar digest[16];
       uschar hexdigest[33];


@@ -3342,11 +3334,8 @@ while (*filter->pc)
             addr->reply->from = expand_string(US"$local_part@$domain");
           else
             addr->reply->from = from.character;
             addr->reply->from = expand_string(US"$local_part@$domain");
           else
             addr->reply->from = from.character;

-          /* Allocation is larger than necessary, but enough even for split MIME words */
-          buffer_capacity=32+4*subject.length;
-          buffer = store_get(buffer_capacity, is_tainted(subject.character));

          /* deconst cast safe as we pass in a non-const item */
          /* deconst cast safe as we pass in a non-const item */

-          addr->reply->subject = US parse_quote_2047(subject.character, subject.length, US"utf-8", buffer, buffer_capacity, TRUE);
+          addr->reply->subject = US parse_quote_2047(subject.character, subject.length, US"utf-8", TRUE);

           addr->reply->oncelog = string_from_gstring(once);
           addr->reply->once_repeat=days*86400;
 
           addr->reply->oncelog = string_from_gstring(once);
           addr->reply->once_repeat=days*86400;
 

diff --git a/test/stdout/0002 b/test/stdout/0002
index b8ff36122c53752ec4bc3a0499e206f2f90c1b54..e12e5690a9e3ce8c9cced3c8e75f262696466eee 100644 (file)
--- a/test/stdout/0002
+++ b/test/stdout/0002
@@ -684,8 +684,8 @@ newline     tab\134backslash ~tilde\177DEL\200\201.
 > abcd      abcd
 > <:abcd:>  =?iso-8859-8?Q?=3C=3Aabcd=3A=3E?=
 > <:ab cd:> =?iso-8859-8?Q?=3C=3Aab_cd=3A=3E?=
 > abcd      abcd
 > <:abcd:>  =?iso-8859-8?Q?=3C=3Aabcd=3A=3E?=
 > <:ab cd:> =?iso-8859-8?Q?=3C=3Aab_cd=3A=3E?=

-> long:     =?iso-8859-8?Q?_here_we_go=3A_a_string_that_is_going_to_be_encoded=3A_?= =?iso-8859-8?Q?it_will_go_over_the_75-char_limit?=
-> long:     =?iso-8859-8?Q?_here_we_go=3A_a_string_that_is_going_to_be_encoded=3A_?= =?iso-8859-8?Q?it_will_go_over_the_75-char_limit_by_a_long_way=3B_in?= =?iso-8859-8?Q?_fact_this_one_will_go_over_the_150_character_limit?=
+> long:     =?iso-8859-8?Q?_here_we_go=3A_a_string_that_is_going_to_be_encoded=3A_it_will_go_ov?= =?iso-8859-8?Q?er_the_75-char_limit?=
+> long:     =?iso-8859-8?Q?_here_we_go=3A_a_string_that_is_going_to_be_encoded=3A_it_will_go_ov?= =?iso-8859-8?Q?er_the_75-char_limit_by_a_long_way=3B_in_fact_this_on?= =?iso-8859-8?Q?e_will_go_over_the_150_character_limit?=

 > 
 > # RFC 2047 decode
 > 
 > 
 > # RFC 2047 decode
 >