SECURITY: length limits on many cmdline options

We'll also now abort upon, rather than silently truncate, a driver name
(router, transport, ACL, etc) encountered in the config which is longer than
the 64-char limit.

(cherry picked from commit ff8bef9ae2370db4a7873fe2ce573a607fe6999f)
(cherry picked from commit a8bd24b96c2027fd839f95a9e6b3282453ae288e)

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 9b8c92bbd66e0c164d7f2be67a3397e448c9eddb..61abb70c0751b8f368f1706f75a38a979a22aea4 100644 (file)
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -51,6 +51,8 @@
 .set ACL "access control lists (ACLs)"
 .set I   "    "
 
 .set ACL "access control lists (ACLs)"
 .set I   "    "
 

+.set drivernamemax "64"
+

 .macro copyyear
 2020
 .endmacro
 .macro copyyear
 2020
 .endmacro


@@ -18802,6 +18804,11 @@ which the preconditions are tested. The order of expansion of the options that
 provide data for a transport is: &%errors_to%&, &%headers_add%&,
 &%headers_remove%&, &%transport%&.
 
 provide data for a transport is: &%errors_to%&, &%headers_add%&,
 &%headers_remove%&, &%transport%&.
 

+.new
+The name of a router is limited to be &drivernamemax; ASCII characters long;
+prior to Exim 4.95 names would be silently truncated at this length, but now
+it is enforced.
+.wen

 
 
 .option address_data routers string&!! unset
 
 
 .option address_data routers string&!! unset


@@ -22345,6 +22352,12 @@ and &$original_domain$& is never set.
 .scindex IIDgenoptra1 "generic options" "transport"
 .scindex IIDgenoptra2 "options" "generic; for transports"
 .scindex IIDgenoptra3 "transport" "generic options for"
 .scindex IIDgenoptra1 "generic options" "transport"
 .scindex IIDgenoptra2 "options" "generic; for transports"
 .scindex IIDgenoptra3 "transport" "generic options for"

+.new
+The name of a transport is limited to be &drivernamemax; ASCII characters long;
+prior to Exim 4.95 names would be silently truncated at this length, but now
+it is enforced.
+.wen
+

 The following generic options apply to all transports:
 
 
 The following generic options apply to all transports:
 
 


@@ -27181,6 +27194,12 @@ permitted to use it as a relay. SMTP authentication is not of relevance to the
 transfer of mail between servers that have no managerial connection with each
 other.
 
 transfer of mail between servers that have no managerial connection with each
 other.
 

+.new
+The name of an authenticator is limited to be &drivernamemax; ASCII characters long;
+prior to Exim 4.95 names would be silently truncated at this length, but now
+it is enforced.
+.wen
+

 .cindex "AUTH" "description of"
 .cindex "ESMTP extensions" AUTH
 Very briefly, the way SMTP authentication works is as follows:
 .cindex "AUTH" "description of"
 .cindex "ESMTP extensions" AUTH
 Very briefly, the way SMTP authentication works is as follows:

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 58ba70f0273486026ea504e063da48226ebaaa98..4c6eb810e269e7aa2832daa7721f42ea82da0d71 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -263,6 +263,12 @@ PP/02 Bug 2643: Correct TLS DH constants.
       incorrect Diffie-Hellman constants in the Exim source.
       Reported by kylon94, code-gen tool fix by Simon Arlott.
 
       incorrect Diffie-Hellman constants in the Exim source.
       Reported by kylon94, code-gen tool fix by Simon Arlott.
 

+PP/03 Fix Linux security issue CVE-2020-SLCWD and guard against PATH_MAX
+      better.  Reported by Qualys.
+
+PP/04 Impose security length checks on various command-line options.
+      Fixes CVE-2020-SPRSS reported by Qualys.
+

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

diff --git a/src/src/acl.c b/src/src/acl.c
index a6c3d4cbed79dea447a648bf700f36652ca317dd..f358516a1983afda619ef316850a08050db4b948 100644 (file)
--- a/src/src/acl.c
+++ b/src/src/acl.c
@@ -486,7 +486,7 @@ static control_def controls_list[] = {
   { US"no_delay_flush",          FALSE,
          ACL_BIT_NOTSMTP | ACL_BIT_NOTSMTP_START
   },
   { US"no_delay_flush",          FALSE,
          ACL_BIT_NOTSMTP | ACL_BIT_NOTSMTP_START
   },

-  
+

 [CONTROL_NO_ENFORCE_SYNC] =
   { US"no_enforce_sync",         FALSE,
          ACL_BIT_NOTSMTP | ACL_BIT_NOTSMTP_START
 [CONTROL_NO_ENFORCE_SYNC] =
   { US"no_enforce_sync",         FALSE,
          ACL_BIT_NOTSMTP | ACL_BIT_NOTSMTP_START


@@ -744,7 +744,7 @@ while ((s = (*func)()))
   int v, c;
   BOOL negated = FALSE;
   uschar *saveline = s;
   int v, c;
   BOOL negated = FALSE;
   uschar *saveline = s;

-  uschar name[64];
+  uschar name[EXIM_DRIVERNAME_MAX];

 
   /* Conditions (but not verbs) are allowed to be negated by an initial
   exclamation mark. */
 
   /* Conditions (but not verbs) are allowed to be negated by an initial
   exclamation mark. */

diff --git a/src/src/exim.c b/src/src/exim.c
index fd1d87362dc924c4363b10e8ea6807af5fece4c6..112fa4f3d5f3856dc329977372c7424dc266e121 100644 (file)
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -760,6 +760,25 @@ vfprintf(stderr, fmt, ap);
 exit(EXIT_FAILURE);
 }
 
 exit(EXIT_FAILURE);
 }
 

+/* fail if a length is too long */
+static void
+exim_len_fail_toolong(int itemlen, int maxlen, const char *description)
+{
+if (itemlen <= maxlen)
+  return;
+fprintf(stderr, "exim: length limit exceeded (%d > %d) for: %s\n",
+        len, maxlen, description)
+exit(EXIT_FAILURE);
+}
+
+/* only pass through the string item back to the caller if it's short enough */
+static const uschar *
+exim_str_fail_toolong(const uschar *item, int maxlen, const char *description)
+{
+exim_len_fail_toolong(Ustrlen(item), maxlen, description);
+return item;
+}
+

 /* exim_chown_failure() called from exim_chown()/exim_fchown() on failure
 of chown()/fchown().  See src/functions.h for more explanation */
 int
 /* exim_chown_failure() called from exim_chown()/exim_fchown() on failure
 of chown()/fchown().  See src/functions.h for more explanation */
 int


@@ -2140,10 +2159,10 @@ on the second character (the one after '-'), to save some effort. */
            {
            if (++i >= argc)
              exim_fail("exim: string expected after %s\n", arg);
            {
            if (++i >= argc)
              exim_fail("exim: string expected after %s\n", arg);

-           if (Ustrcmp(argrest, "d") == 0) ftest_domain = argv[i];
-           else if (Ustrcmp(argrest, "l") == 0) ftest_localpart = argv[i];
-           else if (Ustrcmp(argrest, "p") == 0) ftest_prefix = argv[i];
-           else if (Ustrcmp(argrest, "s") == 0) ftest_suffix = argv[i];
+           if (Ustrcmp(argrest, "d") == 0) ftest_domain = exim_str_fail_toolong(argv[i], EXIM_DOMAINNAME_MAX, "-bfd");
+           else if (Ustrcmp(argrest, "l") == 0) ftest_localpart = exim_str_fail_toolong(argv[i], EXIM_LOCALPART_MAX, "-bfl");
+           else if (Ustrcmp(argrest, "p") == 0) ftest_prefix = exim_str_fail_toolong(argv[i], EXIM_LOCALPART_MAX, "-bfp");
+           else if (Ustrcmp(argrest, "s") == 0) ftest_suffix = exim_str_fail_toolong(argv[i], EXIM_LOCALPART_MAX, "-bfs");

            else badarg = TRUE;
            }
          break;
            else badarg = TRUE;
            }
          break;


@@ -2153,7 +2172,7 @@ on the second character (the one after '-'), to save some effort. */
          if (!*argrest || Ustrcmp(argrest, "c") == 0)
            {
            if (++i >= argc) { badarg = TRUE; break; }
          if (!*argrest || Ustrcmp(argrest, "c") == 0)
            {
            if (++i >= argc) { badarg = TRUE; break; }

-           sender_host_address = string_copy_taint(argv[i], TRUE);
+           sender_host_address = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_IPADDR_MAX, "-bh"), TRUE);

            host_checking = checking = f.log_testing_mode = TRUE;
            f.host_checking_callout = *argrest == 'c';
            message_logs = FALSE;
            host_checking = checking = f.log_testing_mode = TRUE;
            f.host_checking_callout = *argrest == 'c';
            message_logs = FALSE;


@@ -2611,7 +2630,7 @@ on the second character (the one after '-'), to save some effort. */
     case 'F':
     if (!*argrest)
       if (++i < argc) argrest = argv[i]; else { badarg = TRUE; break; }
     case 'F':
     if (!*argrest)
       if (++i < argc) argrest = argv[i]; else { badarg = TRUE; break; }

-    originator_name = string_copy_taint(argrest, TRUE);
+    originator_name = string_copy_taint(exim_str_fail_toolong(argrest, EXIM_HUMANNAME_MAX, "-F"), TRUE);

     f.sender_name_forced = TRUE;
     break;
 
     f.sender_name_forced = TRUE;
     break;
 


@@ -2637,6 +2656,7 @@ on the second character (the one after '-'), to save some effort. */
       uschar *errmess;
       if (!*argrest)
         if (i+1 < argc) argrest = argv[++i]; else { badarg = TRUE; break; }
       uschar *errmess;
       if (!*argrest)
         if (i+1 < argc) argrest = argv[++i]; else { badarg = TRUE; break; }

+      (void) exim_str_fail_toolong(argrest, EXIM_DISPLAYMAIL_MAX, "-f");

       if (!*argrest)
         *(sender_address = store_get(1, FALSE)) = '\0';  /* Ensure writeable memory */
       else
       if (!*argrest)
         *(sender_address = store_get(1, FALSE)) = '\0';  /* Ensure writeable memory */
       else


@@ -2733,9 +2753,9 @@ on the second character (the one after '-'), to save some effort. */
       if (msg_action_arg >= 0)
         exim_fail("exim: incompatible arguments\n");
 
       if (msg_action_arg >= 0)
         exim_fail("exim: incompatible arguments\n");
 

-      continue_transport = string_copy_taint(argv[++i], TRUE);
-      continue_hostname = string_copy_taint(argv[++i], TRUE);
-      continue_host_address = string_copy_taint(argv[++i], TRUE);
+      continue_transport = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_DRIVERNAME_MAX, "-C internal transport"), TRUE);
+      continue_hostname = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_HOSTNAME_MAX, "-C internal hostname"), TRUE);
+      continue_host_address = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_IPADDR_MAX, "-C internal hostaddr"), TRUE);

       continue_sequence = Uatoi(argv[++i]);
       msg_action = MSG_DELIVER;
       msg_action_arg = ++i;
       continue_sequence = Uatoi(argv[++i]);
       msg_action = MSG_DELIVER;
       msg_action_arg = ++i;


@@ -2780,7 +2800,7 @@ on the second character (the one after '-'), to save some effort. */
     /* -MCd: for debug, set a process-purpose string */
 
        case 'd': if (++i < argc)
     /* -MCd: for debug, set a process-purpose string */
 
        case 'd': if (++i < argc)

-                   process_purpose = string_copy_taint(argv[i], TRUE);
+                   process_purpose = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_DRIVERNAME_MAX, "-MCd"), TRUE);

                  else badarg = TRUE;
                  break;
 
                  else badarg = TRUE;
                  break;
 


@@ -2788,7 +2808,7 @@ on the second character (the one after '-'), to save some effort. */
        from the commandline should be tainted - but we will need an untainted
        value for the spoolfile when doing a -odi delivery process. */
 
        from the commandline should be tainted - but we will need an untainted
        value for the spoolfile when doing a -odi delivery process. */
 

-       case 'G': if (++i < argc) queue_name = string_copy_taint(argv[i], FALSE);
+       case 'G': if (++i < argc) queue_name = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_DRIVERNAME_MAX, "-MCG"), FALSE);

                  else badarg = TRUE;
                  break;
 
                  else badarg = TRUE;
                  break;
 


@@ -2861,7 +2881,7 @@ on the second character (the one after '-'), to save some effort. */
        case 'r':
        case 's': if (++i < argc)
                    {
        case 'r':
        case 's': if (++i < argc)
                    {

-                   continue_proxy_sni = string_copy_taint(argv[i], TRUE);
+                   continue_proxy_sni = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_HOSTNAME_MAX, "-MCr/-MCs"), TRUE);

                    if (argrest[1] == 'r') continue_proxy_dane = TRUE;
                    }
                  else badarg = TRUE;
                    if (argrest[1] == 'r') continue_proxy_dane = TRUE;
                    }
                  else badarg = TRUE;


@@ -2873,13 +2893,13 @@ on the second character (the one after '-'), to save some effort. */
     and the TLS cipher. */
 
        case 't': if (++i < argc)
     and the TLS cipher. */
 
        case 't': if (++i < argc)

-                   sending_ip_address = string_copy_taint(argv[i], TRUE);
+                   sending_ip_address = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_IPADDR_MAX, "-MCt IP"), TRUE);

                  else badarg = TRUE;
                  if (++i < argc)
                    sending_port = (int)(Uatol(argv[i]));
                  else badarg = TRUE;
                  if (++i < argc)
                  else badarg = TRUE;
                  if (++i < argc)
                    sending_port = (int)(Uatol(argv[i]));
                  else badarg = TRUE;
                  if (++i < argc)

-                   continue_proxy_cipher = string_copy_taint(argv[i], TRUE);
+                   continue_proxy_cipher = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_CIPHERNAME_MAX, "-MCt cipher"), TRUE);

                  else badarg = TRUE;
                  /*FALLTHROUGH*/
 
                  else badarg = TRUE;
                  /*FALLTHROUGH*/
 


@@ -2906,6 +2926,7 @@ on the second character (the one after '-'), to save some effort. */
     following options which are followed by a single message id, and which
     act on that message. Some of them use the "recipient" addresses as well.
        -Mar  add recipient(s)
     following options which are followed by a single message id, and which
     act on that message. Some of them use the "recipient" addresses as well.
        -Mar  add recipient(s)

+       -MG   move to a different queue

        -Mmad mark all recipients delivered
        -Mmd  mark recipients(s) delivered
        -Mes  edit sender
        -Mmad mark all recipients delivered
        -Mmd  mark recipients(s) delivered
        -Mes  edit sender


@@ -2941,7 +2962,7 @@ on the second character (the one after '-'), to save some effort. */
    else if (Ustrcmp(argrest, "G") == 0)
       {
       msg_action = MSG_SETQUEUE;
    else if (Ustrcmp(argrest, "G") == 0)
       {
       msg_action = MSG_SETQUEUE;

-      queue_name_dest = string_copy_taint(argv[++i], TRUE);
+      queue_name_dest = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_DRIVERNAME_MAX, "-MG"), TRUE);

       }
     else if (Ustrcmp(argrest, "mad") == 0)
       {
       }
     else if (Ustrcmp(argrest, "mad") == 0)
       {


@@ -3154,27 +3175,27 @@ on the second character (the one after '-'), to save some effort. */
        /* -oMa: Set sender host address */
 
        if (Ustrcmp(argrest, "a") == 0)
        /* -oMa: Set sender host address */
 
        if (Ustrcmp(argrest, "a") == 0)

-         sender_host_address = string_copy_taint(argv[++i], TRUE);
+         sender_host_address = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_IPADDR_MAX, "-oMa"), TRUE);

 
        /* -oMaa: Set authenticator name */
 
        else if (Ustrcmp(argrest, "aa") == 0)
 
        /* -oMaa: Set authenticator name */
 
        else if (Ustrcmp(argrest, "aa") == 0)

-         sender_host_authenticated = string_copy_taint(argv[++i], TRUE);
+         sender_host_authenticated = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_DRIVERNAME_MAX, "-oMaa"), TRUE);

 
        /* -oMas: setting authenticated sender */
 
        else if (Ustrcmp(argrest, "as") == 0)
 
        /* -oMas: setting authenticated sender */
 
        else if (Ustrcmp(argrest, "as") == 0)

-         authenticated_sender = string_copy_taint(argv[++i], TRUE);
+         authenticated_sender = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_EMAILADDR_MAX, "-oMas"), TRUE);

 
        /* -oMai: setting authenticated id */
 
        else if (Ustrcmp(argrest, "ai") == 0)
 
        /* -oMai: setting authenticated id */
 
        else if (Ustrcmp(argrest, "ai") == 0)

-         authenticated_id = string_copy_taint(argv[++i], TRUE);
+         authenticated_id = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_EMAILADDR_MAX, "-oMas"), TRUE);

 
        /* -oMi: Set incoming interface address */
 
        else if (Ustrcmp(argrest, "i") == 0)
 
        /* -oMi: Set incoming interface address */
 
        else if (Ustrcmp(argrest, "i") == 0)

-         interface_address = string_copy_taint(argv[++i], TRUE);
+         interface_address = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_IPADDR_MAX, "-oMi"), TRUE);

 
        /* -oMm: Message reference */
 
 
        /* -oMm: Message reference */
 


@@ -3194,19 +3215,19 @@ on the second character (the one after '-'), to save some effort. */
          if (received_protocol)
            exim_fail("received_protocol is set already\n");
          else
          if (received_protocol)
            exim_fail("received_protocol is set already\n");
          else

-           received_protocol = string_copy_taint(argv[++i], TRUE);
+           received_protocol = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_DRIVERNAME_MAX, "-oMr"), TRUE);

 
        /* -oMs: Set sender host name */
 
        else if (Ustrcmp(argrest, "s") == 0)
 
        /* -oMs: Set sender host name */
 
        else if (Ustrcmp(argrest, "s") == 0)

-         sender_host_name = string_copy_taint(argv[++i], TRUE);
+         sender_host_name = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_HOSTNAME_MAX, "-oMs"), TRUE);

 
        /* -oMt: Set sender ident */
 
        else if (Ustrcmp(argrest, "t") == 0)
          {
          sender_ident_set = TRUE;
 
        /* -oMt: Set sender ident */
 
        else if (Ustrcmp(argrest, "t") == 0)
          {
          sender_ident_set = TRUE;

-         sender_ident = string_copy_taint(argv[++i], TRUE);
+         sender_ident = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_IDENTUSER_MAX, "-oMt"), TRUE);

          }
 
        /* Else a bad argument */
          }
 
        /* Else a bad argument */


@@ -3256,10 +3277,11 @@ on the second character (the one after '-'), to save some effort. */
        break;
 
       /* -oX <list>: Override local_interfaces and/or default daemon ports */
        break;
 
       /* -oX <list>: Override local_interfaces and/or default daemon ports */

+      /* Limits: Is there a real limit we want here?  1024 is very arbitrary. */

 
       case 'X':
        if (*argrest) badarg = TRUE;
 
       case 'X':
        if (*argrest) badarg = TRUE;

-       else override_local_interfaces = string_copy_taint(argv[++i], TRUE);
+       else override_local_interfaces = string_copy_taint(exim_str_fail_toolong(argv[++i], 1024, "-oX", TRUE);

        break;
 
       /* -oY: Override creation of daemon notifier socket */
        break;
 
       /* -oY: Override creation of daemon notifier socket */


@@ -3307,9 +3329,10 @@ on the second character (the one after '-'), to save some effort. */
         exim_fail("received_protocol is set already\n");
 
       if (!hn)
         exim_fail("received_protocol is set already\n");
 
       if (!hn)

-        received_protocol = string_copy_taint(argrest, TRUE);
+        received_protocol = string_copy_taint(exim_str_fail_toolong(argrest, EXIM_DRIVERNAME_MAX, "-p<protocol>"), TRUE);

       else
         {
       else
         {

+        (void) exim_str_fail_toolong(argrest, (EXIM_DRIVERNAME_MAX+1+EXIM_HOSTNAME_MAX), "-p<protocol>:<host>");

         received_protocol = string_copyn_taint(argrest, hn - argrest, TRUE);
         sender_host_name = string_copy_taint(hn + 1, TRUE);
         }
         received_protocol = string_copyn_taint(argrest, hn - argrest, TRUE);
         sender_host_name = string_copy_taint(hn + 1, TRUE);
         }


@@ -3365,6 +3388,7 @@ on the second character (the one after '-'), to save some effort. */
       {
       int i;
       for (argrest++, i = 0; argrest[i] && argrest[i] != '/'; ) i++;
       {
       int i;
       for (argrest++, i = 0; argrest[i] && argrest[i] != '/'; ) i++;

+      exim_len_fail_toolong(i, EXIM_DRIVERNAME_MAX, "-q*G<name>");

       queue_name = string_copyn(argrest, i);
       argrest += i;
       if (*argrest == '/') argrest++;
       queue_name = string_copyn(argrest, i);
       argrest += i;
       if (*argrest == '/') argrest++;


@@ -3418,15 +3442,18 @@ on the second character (the one after '-'), to save some effort. */
     /* -R: Set string to match in addresses for forced queue run to
     pick out particular messages. */
 
     /* -R: Set string to match in addresses for forced queue run to
     pick out particular messages. */
 

+    /* Avoid attacks from people providing very long strings, and do so before
+    we make copies. */
+    const char *tainted_selectstr;

     if (*argrest)
     if (*argrest)

-      deliver_selectstring = string_copy_taint(argrest, TRUE);
+      tainted_selectstr = argrest;

     else if (i+1 < argc)
     else if (i+1 < argc)

-      deliver_selectstring = string_copy_taint(argv[++i], TRUE);
+      tainted_selectstr = argv[++i];

     else
       exim_fail("exim: string expected after -R\n");
     else
       exim_fail("exim: string expected after -R\n");

+    deliver_selectstring = string_copy_taint(exim_str_fail_toolong(tainted_selectstr, EXIM_EMAILADDR_MAX, "-R"), TRUE);

     break;
 
     break;
 

-

     /* -r: an obsolete synonym for -f (see above) */
 
 
     /* -r: an obsolete synonym for -f (see above) */
 
 


@@ -3457,12 +3484,14 @@ on the second character (the one after '-'), to save some effort. */
     /* -S: Set string to match in addresses for forced queue run to
     pick out particular messages. */
 
     /* -S: Set string to match in addresses for forced queue run to
     pick out particular messages. */
 

+    const char *tainted_selectstr;

     if (*argrest)
     if (*argrest)

-      deliver_selectstring_sender = string_copy_taint(argrest, TRUE);
+      tainted_selectstr = argrest;

     else if (i+1 < argc)
     else if (i+1 < argc)

-      deliver_selectstring_sender = string_copy_taint(argv[++i], TRUE);
+      tainted_selectstr = argv[++i];

     else
       exim_fail("exim: string expected after -S\n");
     else
       exim_fail("exim: string expected after -S\n");

+    deliver_selectstring_sender = string_copy_taint(exim_str_fail_toolong(tainted_selectstr, EXIM_EMAILADDR_MAX, "-S"), TRUE);

     break;
 
     /* -Tqt is an option that is exclusively for use by the testing suite.
     break;
 
     /* -Tqt is an option that is exclusively for use by the testing suite.


@@ -3544,10 +3573,12 @@ on the second character (the one after '-'), to save some effort. */
         exim_fail("exim: string expected after -X\n");
     break;
 
         exim_fail("exim: string expected after -X\n");
     break;
 

+    /* -z: a line of text to log */
+

     case 'z':
     if (!*argrest)
       if (++i < argc)
     case 'z':
     if (!*argrest)
       if (++i < argc)

-       log_oneline = string_copy_taint(argv[i], TRUE);
+       log_oneline = string_copy_taint(exim_str_fail_toolong(argv[i], 2048, "-z logtext"), TRUE);

       else
         exim_fail("exim: file name expected after %s\n", argv[i-1]);
     break;
       else
         exim_fail("exim: file name expected after %s\n", argv[i-1]);
     break;

diff --git a/src/src/exim.h b/src/src/exim.h
index bb8e4f6e24f8d7b3df4d0fc652c182f40f96357c..8bbeecb4de4748b6fed1174ecdc1e5951c06ed57 100644 (file)
--- a/src/src/exim.h
+++ b/src/src/exim.h
@@ -135,6 +135,51 @@ making unique names. */
 # endif
 #endif
 
 # endif
 #endif
 

+/* RFC 5321 specifies that the maximum length of a local-part is 64 octets
+and the maximum length of a domain is 255 octets, but then also defines
+the maximum length of a forward/reverse path as 256 not 64+1+255.
+For an IP address, the maximum is 45 without a scope and we don't work
+with scoped addresses, so go with that.  (IPv6 with mapped IPv4).
+
+A hostname maximum length is in practice the same as the domainname, for
+the same core reasons (maximum length of a DNS name), but the semantics
+are different and seeing "DOMAIN" in source is confusing when talking about
+hostnames; so we define a second macro.  We'll use RFC 2181 as the reference
+for this one.
+
+There is no known (to me) specification on the maximum length of a human name
+in email addresses and we should be careful about imposing such a limit on
+received email, but in terms of limiting what untrusted callers specify, or
+local generation, having a limit makes sense.  Err on the side of generosity.
+
+For a display mail address, we have a human name, an email in brackets,
+possibly some (Comments), so it needs to be at least 512+3 and some more to
+avoid extraneous errors.
+Since the sane SMTP line length limit is 998, constraining such parameters to
+be 1024 seems generous and unlikely to spuriously reject legitimate
+invocations.
+
+The driver name is a name of a router/transport/authenticator etc in the
+configuration file.  We also use this for some other short strings, such
+as queue names.
+Also TLS ciphersuite name (no real known limit since the protocols use
+integers, but max seen in reality is 45 octets).
+
+RFC 1413 gives us the 512 limit on IDENT protocol userids.
+*/
+
+#define EXIM_EMAILADDR_MAX     256
+#define EXIM_LOCALPART_MAX      64
+#define EXIM_DOMAINNAME_MAX    255
+#define EXIM_IPADDR_MAX         45
+#define EXIM_HOSTNAME_MAX      255
+#define EXIM_HUMANNAME_MAX     256
+#define EXIM_DISPLAYMAIL_MAX  1024
+#define EXIM_DRIVERNAME_MAX     64
+#define EXIM_CIPHERNAME_MAX     64
+#define EXIM_IDENTUSER_MAX     512
+
+

 #include <sys/types.h>
 #include <sys/file.h>
 #include <dirent.h>
 #include <sys/types.h>
 #include <sys/file.h>
 #include <dirent.h>

diff --git a/src/src/macro_predef.c b/src/src/macro_predef.c
index b0f57be19538aafb6341b824f321397057ec1ecb..5340c5af86d17994a22364a7ffe26d7a5dbdfe1e 100644 (file)
--- a/src/src/macro_predef.c
+++ b/src/src/macro_predef.c
@@ -72,7 +72,7 @@ options_from_list(optionlist * opts, unsigned nopt,
   const uschar * section, uschar * group)
 {
 const uschar * s;
   const uschar * section, uschar * group)
 {
 const uschar * s;

-uschar buf[64];
+uschar buf[EXIM_DRIVERNAME_MAX];

 
 /* The 'previously-defined-substring' rule for macros in config file
 lines is done thus for these builtin macros: we know that the table
 
 /* The 'previously-defined-substring' rule for macros in config file
 lines is done thus for these builtin macros: we know that the table

diff --git a/src/src/macros.h b/src/src/macros.h
index 43217807f27e8c2da9c830648c60199882e3d404..72856a53b0b73ee89fb17f8a70b9802287512c42 100644 (file)
--- a/src/src/macros.h
+++ b/src/src/macros.h
@@ -180,12 +180,6 @@ written on the spool, it gets read into big_buffer. */
 
 #define LOCAL_SCAN_MAX_RETURN (BIG_BUFFER_SIZE - 24)
 
 
 #define LOCAL_SCAN_MAX_RETURN (BIG_BUFFER_SIZE - 24)
 

-/* A limit to the length of an address. RFC 2821 limits the local part to 64
-and the domain to 255, so this should be adequate, taking into account quotings
-etc. */
-
-#define ADDRESS_MAXLENGTH 512
-

 /* The length of the base names of spool files, which consist of an internal
 message id with a trailing "-H" or "-D" added. */
 
 /* The length of the base names of spool files, which consist of an internal
 message id with a trailing "-H" or "-D" added. */
 

diff --git a/src/src/malware.c b/src/src/malware.c
index 831e9af1f7e091c9327bda35e9e9b3f38f92806b..a6e354bc4d030e3d18957d5fcc028547a3861240 100644 (file)
--- a/src/src/malware.c
+++ b/src/src/malware.c
@@ -109,7 +109,7 @@ features_malware(void)
 {
 const uschar * s;
 uschar * t;
 {
 const uschar * s;
 uschar * t;

-uschar buf[64];
+uschar buf[EXIM_DRIVERNAME_MAX];

 
 spf(buf, sizeof(buf), US"_HAVE_MALWARE_");
 
 
 spf(buf, sizeof(buf), US"_HAVE_MALWARE_");
 

diff --git a/src/src/parse.c b/src/src/parse.c
index 885a01c0d78b4d427eef77193a26e4b7f1a48fb6..18a6df1987ba4d7457f7a2f41d978e4f057cdd50 100644 (file)
--- a/src/src/parse.c
+++ b/src/src/parse.c
@@ -808,11 +808,11 @@ while (isspace(endptr[-1])) endptr--;
 *end = endptr - US mailbox;
 
 /* Although this code has no limitation on the length of address extracted,
 *end = endptr - US mailbox;
 
 /* Although this code has no limitation on the length of address extracted,

-other parts of Exim may have limits, and in any case, RFC 2821 limits local
-parts to 64 and domains to 255, so we do a check here, giving an error if the
-address is ridiculously long. */
+other parts of Exim may have limits, and in any case, RFC 5321 limits email
+addresses to 256, so we do a check here, giving an error if the address is
+ridiculously long. */

 
 

-if (*end - *start > ADDRESS_MAXLENGTH)
+if (*end - *start > EXIM_EMAILADDR_MAX)

   {
   *errorptr = string_sprintf("address is ridiculously long: %.64s...", yield);
   return NULL;
   {
   *errorptr = string_sprintf("address is ridiculously long: %.64s...", yield);
   return NULL;

diff --git a/src/src/readconf.c b/src/src/readconf.c
index e8e310bebffc910c00f92e266c22d9be267f6fe2..8a7b710a271c675a966919ece8bd6183f566634d 100644 (file)
--- a/src/src/readconf.c
+++ b/src/src/readconf.c
@@ -422,7 +422,7 @@ options_from_list(optionlist_config, nelem(optionlist_config), US"MAIN", NULL);
 void
 options_auths(void)
 {
 void
 options_auths(void)
 {

-uschar buf[64];
+uschar buf[EXIM_DRIVERNAME_MAX];

 
 options_from_list(optionlist_auths, optionlist_auths_size, US"AUTHENTICATORS", NULL);
 
 
 options_from_list(optionlist_auths, optionlist_auths_size, US"AUTHENTICATORS", NULL);
 


@@ -439,7 +439,7 @@ for (struct auth_info * ai = auths_available; ai->driver_name[0]; ai++)
 void
 options_logging(void)
 {
 void
 options_logging(void)
 {

-uschar buf[64];
+uschar buf[EXIM_DRIVERNAME_MAX];

 
 for (bit_table * bp = log_options; bp < log_options + log_options_count; bp++)
   {
 
 for (bit_table * bp = log_options; bp < log_options + log_options_count; bp++)
   {


@@ -684,7 +684,7 @@ Returns:       FALSE iff fatal error
 BOOL
 macro_read_assignment(uschar *s)
 {
 BOOL
 macro_read_assignment(uschar *s)
 {

-uschar name[64];
+uschar name[EXIM_DRIVERNAME_MAX];

 int namelen = 0;
 BOOL redef = FALSE;
 macro_item *m;
 int namelen = 0;
 BOOL redef = FALSE;
 macro_item *m;


@@ -1178,15 +1178,25 @@ uschar *
 readconf_readname(uschar *name, int len, uschar *s)
 {
 int p = 0;
 readconf_readname(uschar *name, int len, uschar *s)
 {
 int p = 0;

+BOOL broken = FALSE;

 
 if (isalpha(Uskip_whitespace(&s)))
   while (isalnum(*s) || *s == '_')
     {
     if (p < len-1) name[p++] = *s;
 
 if (isalpha(Uskip_whitespace(&s)))
   while (isalnum(*s) || *s == '_')
     {
     if (p < len-1) name[p++] = *s;

+    else {
+      broken = TRUE;
+      break;
+    }

     s++;
     }
 
 name[p] = 0;
     s++;
     }
 
 name[p] = 0;

+if (broken) {
+  log_write(0, LOG_PANIC_DIE|LOG_CONFIG_IN,
+            "exim item name too long (>%d), unable to use \"%s\" (truncated)",
+            len, name);
+}

 Uskip_whitespace(&s);
 return s;
 }
 Uskip_whitespace(&s);
 return s;
 }


@@ -1353,7 +1363,7 @@ static BOOL *
 get_set_flag(uschar *name, optionlist *oltop, int last, void *data_block)
 {
 optionlist *ol;
 get_set_flag(uschar *name, optionlist *oltop, int last, void *data_block)
 {
 optionlist *ol;

-uschar name2[64];
+uschar name2[EXIM_DRIVERNAME_MAX];

 sprintf(CS name2, "*set_%.50s", name);
 if (!(ol = find_option(name2, oltop, last)))
   log_write(0, LOG_MAIN|LOG_PANIC_DIE,
 sprintf(CS name2, "*set_%.50s", name);
 if (!(ol = find_option(name2, oltop, last)))
   log_write(0, LOG_MAIN|LOG_PANIC_DIE,


@@ -1626,8 +1636,8 @@ uschar *inttype = US"";
 uschar *sptr;
 uschar *s = buffer;
 uschar **str_target;
 uschar *sptr;
 uschar *s = buffer;
 uschar **str_target;

-uschar name[64];
-uschar name2[64];
+uschar name[EXIM_DRIVERNAME_MAX];
+uschar name2[EXIM_DRIVERNAME_MAX];

 
 /* There may be leading spaces; thereafter, we expect an option name starting
 with a letter. */
 
 /* There may be leading spaces; thereafter, we expect an option name starting
 with a letter. */


@@ -2089,7 +2099,7 @@ switch (type)
   case opt_bool_set:
   if (*s != 0)
     {
   case opt_bool_set:
   if (*s != 0)
     {

-    s = readconf_readname(name2, 64, s);
+    s = readconf_readname(name2, EXIM_DRIVERNAME_MAX, s);

     if (strcmpic(name2, US"true") == 0 || strcmpic(name2, US"yes") == 0)
       boolvalue = TRUE;
     else if (strcmpic(name2, US"false") == 0 || strcmpic(name2, US"no") == 0)
     if (strcmpic(name2, US"true") == 0 || strcmpic(name2, US"yes") == 0)
       boolvalue = TRUE;
     else if (strcmpic(name2, US"false") == 0 || strcmpic(name2, US"no") == 0)


@@ -2436,7 +2446,7 @@ void *value;
 uid_t *uidlist;
 gid_t *gidlist;
 uschar *s;
 uid_t *uidlist;
 gid_t *gidlist;
 uschar *s;

-uschar name2[64];
+uschar name2[EXIM_DRIVERNAME_MAX];

 
 if (!ol)
   {
 
 if (!ol)
   {


@@ -3706,7 +3716,7 @@ uschar *buffer;
 
 while ((buffer = get_config_line()))
   {
 
 while ((buffer = get_config_line()))
   {

-  uschar name[64];
+  uschar name[EXIM_DRIVERNAME_MAX];

   uschar *s;
 
   /* Read the first name on the line and test for the start of a new driver. A
   uschar *s;
 
   /* Read the first name on the line and test for the start of a new driver. A


@@ -4234,7 +4244,7 @@ acl_line = get_config_line();
 
 while(acl_line)
   {
 
 while(acl_line)
   {

-  uschar name[64];
+  uschar name[EXIM_DRIVERNAME_MAX];

   tree_node *node;
   uschar *error;
 
   tree_node *node;
   uschar *error;
 

diff --git a/src/src/transport.c b/src/src/transport.c
index d8fd8585523abcd4d25f61401ee538101aaefe28..89252ec7a67566083cf60b5c67906a9bf18d6591 100644 (file)
--- a/src/src/transport.c
+++ b/src/src/transport.c
@@ -959,9 +959,10 @@ if (!(tctx->options & topt_no_headers))
 
   if (tctx->options & topt_add_return_path)
     {
 
   if (tctx->options & topt_add_return_path)
     {

-    uschar buffer[ADDRESS_MAXLENGTH + 20];
-    int n = sprintf(CS buffer, "Return-path: <%.*s>\n", ADDRESS_MAXLENGTH,
-      return_path);
+    uschar buffer[EXIM_EMAILADDR_MAX + 20];
+    int n = string_format(CS buffer, sizeof(buffer),
+                          "Return-path: <%.*s>\n",
+                          EXIM_EMAILADDR_MAX, return_path);

     if (!write_chunk(tctx, buffer, n)) goto bad;
     }
 
     if (!write_chunk(tctx, buffer, n)) goto bad;
     }
 


@@ -1729,7 +1730,7 @@ while (1)
     {
     msgq[i].bKeep = TRUE;
 
     {
     msgq[i].bKeep = TRUE;
 

-    Ustrncpy_nt(msgq[i].message_id, host_record->text + (i * MESSAGE_ID_LENGTH), 
+    Ustrncpy_nt(msgq[i].message_id, host_record->text + (i * MESSAGE_ID_LENGTH),

       MESSAGE_ID_LENGTH);
     msgq[i].message_id[MESSAGE_ID_LENGTH] = 0;
     }
       MESSAGE_ID_LENGTH);
     msgq[i].message_id[MESSAGE_ID_LENGTH] = 0;
     }