From ea31415690291ab5be6c70a20738e5b1291b92dd Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

Date: Mon, 26 Feb 2018 18:09:32 -0500

Subject: [PATCH] Bug 1525628 - CVE-2017-15135: Authentication bypass due to

 lack of size check in slapi_ct_memcmp function in ch_malloc.c

    Description: Backport this fix top 1.2.11(RHEL 6.9)

---

 ldap/servers/plugins/pwdstorage/clear_pwd.c |  4 +-

 ldap/servers/plugins/pwdstorage/crypt_pwd.c |  2 +-

 ldap/servers/plugins/pwdstorage/md5_pwd.c   |  2 +-

 ldap/servers/plugins/pwdstorage/sha_pwd.c   | 16 ++++++--

 ldap/servers/plugins/pwdstorage/smd5_pwd.c  | 60 +++++++++++++++--------------

 ldap/servers/slapd/ch_malloc.c              | 34 ++++++++++++++--

 ldap/servers/slapd/slapi-plugin.h           |  2 +-

 7 files changed, 79 insertions(+), 41 deletions(-)

diff --git a/ldap/servers/plugins/pwdstorage/clear_pwd.c b/ldap/servers/plugins/pwdstorage/clear_pwd.c

index 5a889d4aa..55809a252 100644

--- a/ldap/servers/plugins/pwdstorage/clear_pwd.c

+++ b/ldap/servers/plugins/pwdstorage/clear_pwd.c

@@ -68,7 +68,7 @@ clear_pw_cmp( const char *userpwd, const char *dbpwd )

          * However, even if the first part of userpw matches dbpwd, but len !=, we

          * have already failed anyawy. This prevents substring matching.

          */

-        if (slapi_ct_memcmp(userpwd, dbpwd, len_dbp) != 0) {

+        if (slapi_ct_memcmp(userpwd, dbpwd, len_dbp, len_dbp) != 0) {

             result = 1;

         }

     } else {

@@ -80,7 +80,7 @@ clear_pw_cmp( const char *userpwd, const char *dbpwd )

          * dbpwd to itself. We have already got result == 1 if we are here, so we are

          * just trying to take up time!

          */

-        if (slapi_ct_memcmp(dbpwd, dbpwd, len_dbp)) {

+        if (slapi_ct_memcmp(dbpwd, dbpwd, len_dbp, len_dbp)) {

             /* Do nothing, we have the if to fix a coverity check. */

         }

     }

diff --git a/ldap/servers/plugins/pwdstorage/crypt_pwd.c b/ldap/servers/plugins/pwdstorage/crypt_pwd.c

index 37557deb1..eb498e603 100644

--- a/ldap/servers/plugins/pwdstorage/crypt_pwd.c

+++ b/ldap/servers/plugins/pwdstorage/crypt_pwd.c

@@ -87,7 +87,7 @@ crypt_pw_cmp( const char *userpwd, const char *dbpwd )

     /* we use salt (first 2 chars) of encoded password in call to crypt() */

     cp = crypt( userpwd, dbpwd );

     if (cp) {

-       rc= slapi_ct_memcmp( dbpwd, cp, strlen(dbpwd));

+       rc= slapi_ct_memcmp( dbpwd, cp, strlen(dbpwd), strlen(cp));

     } else {

        rc = -1;

     }

diff --git a/ldap/servers/plugins/pwdstorage/md5_pwd.c b/ldap/servers/plugins/pwdstorage/md5_pwd.c

index d00ef945b..8e352edd3 100644

--- a/ldap/servers/plugins/pwdstorage/md5_pwd.c

+++ b/ldap/servers/plugins/pwdstorage/md5_pwd.c

@@ -86,7 +86,7 @@ md5_pw_cmp( const char *userpwd, const char *dbpwd )

    bver = NSSBase64_EncodeItem(NULL, (char *)b2a_out, sizeof b2a_out, &binary_item);

    /* bver points to b2a_out upon success */

    if (bver) {

-	   rc = slapi_ct_memcmp(bver,dbpwd, strlen(dbpwd));

+	   rc = slapi_ct_memcmp(bver,dbpwd, strlen(dbpwd), strlen(bver));

    } else {

 	   slapi_log_error(SLAPI_LOG_PLUGIN, MD5_SUBSYSTEM_NAME,

 					   "Could not base64 encode hashed value for password compare");

diff --git a/ldap/servers/plugins/pwdstorage/sha_pwd.c b/ldap/servers/plugins/pwdstorage/sha_pwd.c

index 1b787524b..994e20480 100644

--- a/ldap/servers/plugins/pwdstorage/sha_pwd.c

+++ b/ldap/servers/plugins/pwdstorage/sha_pwd.c

@@ -78,7 +78,7 @@ sha_pw_cmp (const char *userpwd, const char *dbpwd, unsigned int shaLen )

     char userhash[MAX_SHA_HASH_SIZE];

     char quick_dbhash[MAX_SHA_HASH_SIZE + SHA_SALT_LENGTH + 3];

     char *dbhash = quick_dbhash;

-    struct berval salt;

+    struct berval salt = {0};

     int hash_len;   /* must be a signed valued -- see below */

     unsigned int secOID;

     char *schemeName;

@@ -150,9 +150,19 @@ sha_pw_cmp (const char *userpwd, const char *dbpwd, unsigned int shaLen )

     /* the proof is in the comparison... */

     if ( hash_len >= shaLen ) {

-        result = slapi_ct_memcmp( userhash, dbhash, shaLen );

+        /*

+         * This say "if the hash has a salt IE >, OR if they are equal, check the hash component ONLY.

+         * This is why we repeat shaLen twice, even though it seems odd. If you have a dbhast of ssha

+         * it's len is 28, and the userpw is 20, but 0 - 20 is the sha, and 21-28 is the salt, which

+         * has already been processed into userhash.

+         * The case where dbpwd is truncated is handled above in "invalid base64" arm.

+         */

+        result = slapi_ct_memcmp(userhash, dbhash, shaLen, shaLen);

     } else {

-        result = slapi_ct_memcmp( userhash, dbhash + OLD_SALT_LENGTH, hash_len - OLD_SALT_LENGTH );

+        /* This case is for if the salt is at the START, which only applies to DS40B1 case.

+         * May never be a valid check...

+         */

+        result = slapi_ct_memcmp(userhash, dbhash + OLD_SALT_LENGTH, shaLen, hash_len - OLD_SALT_LENGTH);

     }

 loser:

diff --git a/ldap/servers/plugins/pwdstorage/smd5_pwd.c b/ldap/servers/plugins/pwdstorage/smd5_pwd.c

index ecf9760a4..5afff6022 100644

--- a/ldap/servers/plugins/pwdstorage/smd5_pwd.c

+++ b/ldap/servers/plugins/pwdstorage/smd5_pwd.c

@@ -81,35 +81,37 @@ smd5_pw_cmp( const char *userpwd, const char *dbpwd )

    /*

     * Decode hash stored in database.

     */

-   hash_len = pwdstorage_base64_decode_len(dbpwd, 0);

-   if ( hash_len >= sizeof(quick_dbhash) ) { /* get more space: */

-      dbhash = (char*) slapi_ch_calloc( hash_len + 1, sizeof(char) );

-      if ( dbhash == NULL ) goto loser;

-   } else {

-      memset( quick_dbhash, 0, sizeof(quick_dbhash) );

-   }

-

-   hashresult = PL_Base64Decode( dbpwd, 0, dbhash );

-   if (NULL == hashresult) {

-      slapi_log_error( SLAPI_LOG_PLUGIN, SALTED_MD5_SUBSYSTEM_NAME,

-            "smd5_pw_cmp: userPassword \"%s\" is the wrong length "

-            "or is not properly encoded BASE64\n", dbpwd );

-      goto loser;

-   }

-

-   salt.bv_val = (void*)(dbhash + MD5_LENGTH); /* salt starts after hash value */

-   salt.bv_len = hash_len - MD5_LENGTH; /* remaining bytes must be salt */

-

-   /* create the hash */

-   memset( userhash, 0, sizeof(userhash) );

-   PK11_DigestBegin(ctx);

-   PK11_DigestOp(ctx, (const unsigned char *)userpwd, strlen(userpwd));

-   PK11_DigestOp(ctx, (unsigned char*)(salt.bv_val), salt.bv_len);

-   PK11_DigestFinal(ctx, userhash, &outLen, sizeof userhash);

-   PK11_DestroyContext(ctx, 1);

-

-   /* Compare everything up to the salt. */

-   rc = slapi_ct_memcmp( userhash, dbhash, MD5_LENGTH );

+    hash_len = pwdstorage_base64_decode_len(dbpwd, 0);

+    if (hash_len >= sizeof(quick_dbhash)) { /* get more space: */

+        dbhash = (char *)slapi_ch_calloc(hash_len + 1, sizeof(char));

+        if (dbhash == NULL)

+            goto loser;

+    } else {

+        memset(quick_dbhash, 0, sizeof(quick_dbhash));

+    }

+

+    hashresult = PL_Base64Decode(dbpwd, 0, dbhash);

+    if (NULL == hashresult) {

+        slapi_log_error(SLAPI_LOG_PLUGIN, SALTED_MD5_SUBSYSTEM_NAME,

+                        "smd5_pw_cmp: userPassword \"%s\" is the wrong length "

+                        "or is not properly encoded BASE64\n",

+                        dbpwd);

+        goto loser;

+    }

+

+    salt.bv_val = (void *)(dbhash + MD5_LENGTH); /* salt starts after hash value */

+    salt.bv_len = hash_len - MD5_LENGTH;         /* remaining bytes must be salt */

+

+    /* create the hash */

+    memset(userhash, 0, sizeof(userhash));

+    PK11_DigestBegin(ctx);

+    PK11_DigestOp(ctx, (const unsigned char *)userpwd, strlen(userpwd));

+    PK11_DigestOp(ctx, (unsigned char *)(salt.bv_val), salt.bv_len);

+    PK11_DigestFinal(ctx, userhash, &outLen, sizeof userhash);

+    PK11_DestroyContext(ctx, 1);

+

+    /* Compare everything up to the salt. */

+    rc = slapi_ct_memcmp(userhash, dbhash, MD5_LENGTH, MD5_LENGTH);

 loser:

    if ( dbhash && dbhash != quick_dbhash ) slapi_ch_free_string( (char **)&dbhash );

diff --git a/ldap/servers/slapd/ch_malloc.c b/ldap/servers/slapd/ch_malloc.c

index 38bca5c18..1da053e3f 100644

--- a/ldap/servers/slapd/ch_malloc.c

+++ b/ldap/servers/slapd/ch_malloc.c

@@ -740,7 +740,7 @@ memory_record_dump( caddr_t data, caddr_t arg )

 /* Constant time memcmp. Does not shortcircuit on failure! */

 /* This relies on p1 and p2 both being size at least n! */

 int

-slapi_ct_memcmp( const void *p1, const void *p2, size_t n)

+slapi_ct_memcmp( const void *p1, const void *p2, size_t n1, size_t n2)

 {

     int result = 0;

     const unsigned char *_p1 = (const unsigned char *)p1;

@@ -751,9 +751,35 @@ slapi_ct_memcmp( const void *p1, const void *p2, size_t n)

         return 2;

     }

-    for (i = 0; i < n; i++) {

-        if (_p1[i] ^ _p2[i]) {

-            result = 1;

+    if (n1 == n2) {

+        for (i = 0; i < n1; i++) {

+            if (_p1[i] ^ _p2[i]) {

+                result = 1;

+            }

+        }

+    } else {

+        const unsigned char *_pa;

+        const unsigned char *_pb;

+        size_t nl;

+        if (n2 > n1) {

+            _pa = _p2;

+            _pb = _p2;

+            nl = n2;

+        } else {

+            _pa = _p1;

+            _pb = _p1;

+            nl = n1;

+        }

+        /* We already fail as n1 != n2 */

+        result = 3;

+        for (i = 0; i < nl; i++) {

+            if (_pa[i] ^ _pb[i]) {

+                /*

+                 * If we don't mutate result here, dead code elimination

+                 * we remove for loop.

+                 */

+                result = 4;

+            }

         }

     }

     return result;

diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h

index 7f361731b..ac040c716 100644

--- a/ldap/servers/slapd/slapi-plugin.h

+++ b/ldap/servers/slapd/slapi-plugin.h

@@ -5531,7 +5531,7 @@ char * slapi_ch_smprintf(const char *fmt, ...)

  * \param n length in bytes of the content of p1 AND p2.

  * \return 0 on match. 1 on non-match. 2 on presence of NULL pointer in p1 or p2.

  */

-int slapi_ct_memcmp( const void *p1, const void *p2, size_t n);

+int slapi_ct_memcmp( const void *p1, const void *p2, size_t n1, size_t n2);

 /*

  * syntax plugin routines

-- 

2.13.6