Fix IP SRR parsing. Bug 3124

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 37cc3b77dfd2b6bff2cb401e5de909b78128d09e..cf9420b1924fb664c6ddb3b55b1185c6e3917605 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -80,6 +80,13 @@ JH/15 Disallow tainted metadata in lists.
       - Exclamation-marks ("!" signifying negation) are not checked for taint
       at this time.
 
       - Exclamation-marks ("!" signifying negation) are not checked for taint
       at this time.
 

+JH/16 Bug 3124: Fix theoretical crash in received connection, triggerable by a
+      crafted packet with massive count of IP options.  A buffer overflow was
+      detected, but a null-deref results.  In practice, IP packets with options
+      are rare (to non-existent).  Exim refuses connections having any, but this
+      issue was in the coding for logging preceding that refusal.  If coredumps
+      were enabled (not common), an attack could cause filesystem space usage.
+

 Exim version 4.98
 -----------------
 
 Exim version 4.98
 -----------------
 

diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index fd28c4454bcf9778c2050af9e2ee3e53c179098e..be065eafbcab10251bad8a10a6da924d5cbf1b77 100644 (file)
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -2317,10 +2317,9 @@ if (!f.sender_host_unknown)
 
     else if (optlen > 0)
       {
 
     else if (optlen > 0)
       {

-      uschar * p = big_buffer;
-      uschar * pend = big_buffer + big_buffer_size;
+      uschar * p = big_buffer, * pend = big_buffer + big_buffer_size;

       uschar * adptr;
       uschar * adptr;

-      int optcount;
+      int optcount, sprint_len;

       struct in_addr addr;
 
 # if OPTSTYLE == 1
       struct in_addr addr;
 
 # if OPTSTYLE == 1


@@ -2351,57 +2350,57 @@ if (!f.sender_host_unknown)
           case IPOPT_LSRR:
            if (!
 # if OPTSTYLE == 1
           case IPOPT_LSRR:
            if (!
 # if OPTSTYLE == 1

-                string_format(p, pend-p, " %s [@%s",
-                (*opt == IPOPT_SSRR)? "SSRR" : "LSRR",
-                inet_ntoa(*((struct in_addr *)(&(ipopt->faddr)))))
+                string_format(p, pend-p, " %s [@%s%n",
+                  *opt == IPOPT_SSRR ? "SSRR" : "LSRR",
+                  inet_ntoa(*(struct in_addr *)&ipopt->faddr),
+                  &sprint_len)

 # elif OPTSTYLE == 2
 # elif OPTSTYLE == 2

-                string_format(p, pend-p, " %s [@%s",
-                (*opt == IPOPT_SSRR)? "SSRR" : "LSRR",
-                inet_ntoa(ipopt->ip_dst))
+                string_format(p, pend-p, " %s [@%s%n",
+                  *opt == IPOPT_SSRR ? "SSRR" : "LSRR",
+                  inet_ntoa(ipopt->ip_dst),
+                  &sprint_len)

 # else
 # else

-                string_format(p, pend-p, " %s [@%s",
-                (*opt == IPOPT_SSRR)? "SSRR" : "LSRR",
-                inet_ntoa(ipopt->ipopt_dst))
+                string_format(p, pend-p, " %s [@%s%n",
+                  *opt == IPOPT_SSRR ? "SSRR" : "LSRR",
+                  inet_ntoa(ipopt->ipopt_dst),
+                  &sprint_len)

 # endif
 # endif

-             )
-             {
+               )

              opt = NULL;
              opt = NULL;

-             break;
-             }
-
-           p += Ustrlen(p);
-           optcount = (opt[1] - 3) / sizeof(struct in_addr);
-           adptr = opt + 3;
-           while (optcount-- > 0)
+           else

              {
              {

-             memcpy(&addr, adptr, sizeof(addr));
-             if (!string_format(p, pend - p - 1, "%s%s",
-                   (optcount == 0)? ":" : "@", inet_ntoa(addr)))
+             p += sprint_len;
+             optcount = (opt[1] - 3) / sizeof(struct in_addr);
+             adptr = opt + 3;
+             while (optcount-- > 0)

                {
                {

-               opt = NULL;
-               break;
+               memcpy(&addr, adptr, sizeof(addr));
+               if (!string_format(p, pend - p - 1, "%s%s%n",
+                     optcount == 0 ? ":" : "@", inet_ntoa(addr), &sprint_len))
+                 { opt = NULL; goto bad_srr; }
+               p += sprint_len;
+               adptr += sizeof(struct in_addr);

                }
                }

-             p += Ustrlen(p);
-             adptr += sizeof(struct in_addr);
+             *p++ = ']';
+             opt += opt[1];

              }
              }

-           *p++ = ']';
-           opt += opt[1];
-           break;
+bad_srr:    break;

 
           default:
 
           default:

+           if (pend - p < 4 + 3*opt[1])
+             opt = NULL;
+           else

              {
              {

-             if (pend - p < 4 + 3*opt[1]) { opt = NULL; break; }
-             Ustrcat(p, "[ ");
-             p += 2;
+             p = Ustpcpy(p, "[ ");

              for (int i = 0; i < opt[1]; i++)
                p += sprintf(CS p, "%2.2x ", opt[i]);
              *p++ = ']';
              for (int i = 0; i < opt[1]; i++)
                p += sprintf(CS p, "%2.2x ", opt[i]);
              *p++ = ']';

+             opt += opt[1];

              }
              }

-           opt += opt[1];

            break;
           }
 
            break;
           }
 

-      *p = 0;
+      *p = '\0';

       log_write(0, LOG_MAIN, "%s", big_buffer);
 
       /* Refuse any call with IP options. This is what tcpwrappers 7.5 does. */
       log_write(0, LOG_MAIN, "%s", big_buffer);
 
       /* Refuse any call with IP options. This is what tcpwrappers 7.5 does. */