Fix taint trap in parse_fix_phrase(). Bug 2617
authorJeremy Harris <jgh146exb@wizmail.org>
Thu, 9 Jul 2020 14:30:55 +0000 (15:30 +0100)
committerJeremy Harris <jgh146exb@wizmail.org>
Thu, 9 Jul 2020 20:07:32 +0000 (21:07 +0100)
(cherry picked from commit 3c90bbcdc7cf73298156f7bcd5f5e750e7814e72)

doc/doc-txt/ChangeLog
src/src/acl.c
src/src/exim.c
src/src/expand.c
src/src/functions.h
src/src/parse.c
src/src/rewrite.c
src/src/sieve.c
test/stdout/0002

index 08c6638a9292dc41b15120e242bdeee60357a17b..a1f39459e728252b4e3a8af5d24c092211d23aa9 100644 (file)
@@ -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.
 
+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
 -----------------
index 62cb6856179e7868c85014e93e35bd3ebbd160f0..105b1b473999b3cae45a9756d058c74a6f3e89cc 100644 (file)
@@ -3199,8 +3199,7 @@ for (; cb; cb = cb->next)
              {
              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;
index dde99106501a25e9dd07da954b7c9860ec05cdd7..ac0ff552317bc14a7bb5c88962737b6509e19733 100644 (file)
@@ -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.*/
 
-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
index 6ed22c14d958a95a80a481f3264b4fd39fbccc4c..791222324b6f261d35aeef8dc9ef47d0f6673b37 100644 (file)
@@ -7571,13 +7571,10 @@ while (*s != 0)
       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,
-                             buffer, sizeof(buffer), FALSE));
+                             FALSE));
         continue;
-        }
 
       /* RFC 2047 decode */
 
index afe9f1bf4883d05c7b7b1c30c20ec109418f2a21..f4d1622dc205fbee34d042e19d08c4a08e9acdd8 100644 (file)
@@ -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 *);
-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 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
index e3b471f1a6e7a017171e00f4668ebc1c9a1e13d6..3ea758ac9a0b18e23d5937268c5554c1ce3dee38 100644 (file)
@@ -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
-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
@@ -856,77 +855,57 @@ Arguments:
                  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
-               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 *
-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;
+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 == ' ')
       {
-      *t++ = '_';
+      g = string_catn(g, US"_", 1);
       first_byte = FALSE;
       }
     else
       {
-      t += sprintf(CS t, "=%02X", ch);
+      g = string_fmt_append(g, "=%02X", ch);
       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".
 
-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
-  buffer       a buffer to put the result in
-  buffer_size  the size of the buffer
 
 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;
+uschar * buffer;
 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. */
@@ -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;
 
-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 */
 
+buffer = store_get(len*4, is_tainted(phrase));
+
 s = phrase;
 end = s + len;
 yield = t = buffer + 1;
@@ -1173,6 +1147,7 @@ while (s < end)
   }
 
 *t = 0;
+store_release_above(t+1);
 return yield;
 }
 
@@ -2102,7 +2077,6 @@ int main(void)
 {
 int start, end, domain;
 uschar buffer[1024];
-uschar outbuff[1024];
 
 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;
-  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");
index f942bec051a841cca46973b1a49415ab9fd05507..7bff8a27318bd9959ab8e650e5c395bf368ccd6d 100644 (file)
@@ -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 buff1[256], buff2[256];
 
         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++;
-        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);
index 18aa3e609ead7f995d2bcde2bbd63473d0379e79..2038f336b7b1e3b240f45d9bceae35eb3593a7cc 100644 (file)
@@ -3087,11 +3087,8 @@ while (*filter->pc)
             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);
@@ -3104,12 +3101,9 @@ while (*filter->pc)
                 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,
-                 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);
@@ -3263,8 +3257,6 @@ while (*filter->pc)
     if (exec)
       {
       address_item *addr;
-      uschar *buffer;
-      int buffer_capacity;
       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;
-          /* 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 */
-          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;
 
index b8ff36122c53752ec4bc3a0499e206f2f90c1b54..e12e5690a9e3ce8c9cced3c8e75f262696466eee 100644 (file)
@@ -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?=
-> 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
 >