[exim-cvs] Fix IP SRR parsing. Bug 3124

Gitweb: https://git.exim.org/exim.git/commitdiff/8b8667503b5248524dbe94fa11be312fca5b380c

Commit:     8b8667503b5248524dbe94fa11be312fca5b380c
Parent:     e2725fc0da9c39d279a883cbd184cf3243026dd3
Author:     Jeremy Harris <jgh146exb@???>
AuthorDate: Tue Dec 3 00:13:43 2024 +0000
Committer:  Jeremy Harris <jgh146exb@???>
CommitDate: Tue Dec 3 00:18:12 2024 +0000

    Fix IP SRR parsing.  Bug 3124
---
 doc/doc-txt/ChangeLog |  7 +++++
 src/src/smtp_in.c     | 71 +++++++++++++++++++++++++--------------------------
 2 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 37cc3b77d..cf9420b19 100644
--- 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.

+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
 -----------------

diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index fd28c4454..be065eafb 100644
--- 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)
       {
-      uschar * p = big_buffer;
-      uschar * pend = big_buffer + big_buffer_size;
+      uschar * p = big_buffer, * pend = big_buffer + big_buffer_size;
       uschar * adptr;
-      int optcount;
+      int optcount, sprint_len;
       struct in_addr addr;

 # if OPTSTYLE == 1
@@ -2351,57 +2350,57 @@ if (!f.sender_host_unknown)
           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
-             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
-             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
-          )
-          {
+            )
           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:
+        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++ = ']';
+          opt += opt[1];
           }
-        opt += opt[1];
         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. */

--
## subscription configuration (requires account):
## https://lists.exim.org/mailman3/postorius/lists/exim-cvs.lists.exim.org/
## unsubscribe (doesn't require an account):
## exim-cvs-unsubscribe@???
## Exim details at http://www.exim.org/
## Please use the Wiki with this list - http://wiki.exim.org/