Package: libdkim0d
Version: 1:1.0.19-3
Severity: grave
Tags: security
Justification: user security hole


The following patch makes libdkim use strtok_r() instead of strtok() for thread
safety.

If a server process has multiple threads operating on behalf of different users
while verifying dkim data (IE receiving mail for multiple users) then data may
be leaked.  Also this may cause a server process to crash and there is a
possibility of it being an exploitable bug.

I don't think that the security concerns are great enough to make a secret
report for a coordinated release, but I do think that they are great enough
to justify a grave bug report.

diff -ru libdkim-1.0.19/src/dkimverify.cpp libdkim-1.0.19-new/src/dkimverify.cpp
--- libdkim-1.0.19/src/dkimverify.cpp   2008-05-12 20:08:06.000000000 +1000
+++ libdkim-1.0.19-new/src/dkimverify.cpp       2009-06-11 18:28:10.000000000 
+1000
@@ -855,6 +855,9 @@
 
////////////////////////////////////////////////////////////////////////////////
 int CDKIMVerify::ParseDKIMSignature( const string& sHeader, SignatureInfo &sig 
)
 {
+       // for strtok_r()
+       char *saveptr;
+
        // save header for later
        sig.Header = sHeader;
 
@@ -1032,7 +1035,7 @@
        {
                // make sure "dns" is in the list
                bool HasDNS = false;
-               char *s = strtok(values[9], ":");
+               char *s = strtok_r(values[9], ":", &saveptr);
                while (s != NULL)
                {
                        if (strncmp(s, "dns", 3) == 0 && (s[3] == '\0' || s[3] 
== '/'))
@@ -1040,7 +1043,7 @@
                                HasDNS = true;
                                break;
                        }
-                       s = strtok(NULL, ": \t");
+                       s = strtok_r(NULL, ": \t", &saveptr);
                }
                if (!HasDNS)
                        return DKIM_BAD_SYNTAX;         // todo: maybe create a 
new error code for unknown query method
@@ -1080,7 +1083,7 @@
        // parse the signed headers list
        bool HasFrom = false, HasSubject = false;
        RemoveSWSP(values[4]);                  // header names shouldn't have 
spaces in them so this should be ok...
-       char *s = strtok(values[4], ":");
+       char *s = strtok_r(values[4], ":", &saveptr);
        while (s != NULL)
        {
                if (_stricmp(s, "From") == 0)
@@ -1090,7 +1093,7 @@
 
                sig.SignedHeaders.push_back(s);
 
-               s = strtok(NULL, ":");
+               s = strtok_r(NULL, ":", &saveptr);
        }
 
        if (!HasFrom)
@@ -1194,6 +1197,9 @@
 
////////////////////////////////////////////////////////////////////////////////
 int SelectorInfo::Parse( char* Buffer )
 {
+       // for strtok_r()
+       char *saveptr;
+
        static const char *tags[] = {"v","g","h","k","p","s","t","n",NULL};
        char *values[sizeof(tags)/sizeof(tags[0])] = {NULL};
 
@@ -1235,14 +1241,14 @@
        else
        {
                // MUST include "sha1" or "sha256"
-               char *s = strtok(values[2], ":");
+               char *s = strtok_r(values[2], ":", &saveptr);
                while (s != NULL)
                {
                        if (strcmp(s, "sha1") == 0)
                                AllowSHA1 = true;
                        else if (strcmp(s, "sha256") == 0)
                                AllowSHA256 = true;
-                       s = strtok(NULL, ":");
+                       s = strtok_r(NULL, ":", &saveptr);
                }
                if ( !(AllowSHA1 || AllowSHA256) )
                        return DKIM_SELECTOR_INVALID;   // todo: maybe create a 
new error code for unsupported hash algorithm
@@ -1261,7 +1267,7 @@
        {
                // make sure "*" or "email" is in the list
                bool ServiceTypeMatch = false;
-               char *s = strtok(values[5], ":");
+               char *s = strtok_r(values[5], ":", &saveptr);
                while (s != NULL)
                {
                        if (strcmp(s, "*") == 0 || strcmp(s, "email") == 0)
@@ -1269,7 +1275,7 @@
                                ServiceTypeMatch = true;
                                break;
                        }
-                       s = strtok(NULL, ":");
+                       s = strtok_r(NULL, ":", &saveptr);
                }
                if (!ServiceTypeMatch)
                        return DKIM_SELECTOR_INVALID;
@@ -1278,7 +1284,7 @@
        // flags
        if (values[6] != NULL)
        {
-               char *s = strtok(values[6], ":");
+               char *s = strtok_r(values[6], ":", &saveptr);
                while (s != NULL)
                {
                        if (strcmp(s, "y") == 0)
@@ -1289,7 +1295,7 @@
                        {
                                SameDomain = true;
                        }
-                       s = strtok(NULL, ":");
+                       s = strtok_r(NULL, ":", &saveptr);
                }
        }
 
@@ -1388,6 +1394,9 @@
 
////////////////////////////////////////////////////////////////////////////////
 int CDKIMVerify::GetSSP( const string &sDomain, int &iSSP, bool &bTesting )
 {
+       // for strtok_r()
+       char *saveptr;
+
        string sFQDN = "_ssp._domainkey.";
        sFQDN += sDomain;
 
@@ -1456,7 +1465,7 @@
                        // flags
                        if (values[1] != NULL)
                        {
-                               char *s = strtok(values[1], "|");
+                               char *s = strtok_r(values[1], "|", &saveptr);
                                while (s != NULL)
                                {
                                        if (strcmp(s, "y") == 0)
@@ -1474,7 +1483,7 @@
                                                        return DKIM_SUCCESS;
                                                }
                                        }
-                                       s = strtok(NULL, "|");
+                                       s = strtok_r(NULL, "|", &saveptr);
                                }
                        }
                }

-- System Information:
Debian Release: 5.0.1
  APT prefers stable
  APT policy: (500, 'stable')
Architecture: i386 (i686)

Kernel: Linux 2.6.26-1-686 (SMP w/1 CPU core)
Locale: LANG=en_AU.UTF-8, LC_CTYPE=en_AU.UTF-8 (charmap=ANSI_X3.4-1968) 
(ignored: LC_ALL set to C)
Shell: /bin/sh linked to /bin/bash

Versions of packages libdkim0d depends on:
ii  libc6                   2.7-18           GNU C Library: Shared libraries
ii  libgcc1                 1:4.3.2-1.1      GCC support library
ii  libssl0.9.8             0.9.8g-15+lenny1 SSL shared libraries
ii  libstdc++6              4.3.2-1.1        The GNU Standard C++ Library v3

libdkim0d recommends no packages.

libdkim0d suggests no packages.

-- no debconf information



_______________________________________________
Secure-testing-team mailing list
Secure-testing-team@lists.alioth.debian.org
http://lists.alioth.debian.org/mailman/listinfo/secure-testing-team

Reply via email to