[SCM] Samba Shared Repository - branch v4-17-test updated

Jule Anger janger at samba.org
Tue Nov 15 16:35:24 UTC 2022


The branch, v4-17-test has been updated
       via  3e1f07b1027 VERSION: Bump version up to Samba 4.17.4...
       via  120f7790f6b Merge tag 'samba-4.17.3' into v4-17-test
       via  212ebbf7f4f VERSION: Disable GIT_SNAPSHOT for the 4.17.3 release.
       via  5e5de5dff5c WHATSNEW: Add release notes for Samba 4.17.3.
       via  5d845feca47 CVE-2022-42898 third_party/heimdal: PAC parse integer overflows
      from  2803e76fba0 smbd: Fix Bug 15221

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-17-test


- Log -----------------------------------------------------------------
commit 3e1f07b10276e443746b7a4658c0f6ceca61bc96
Author: Jule Anger <janger at samba.org>
Date:   Tue Nov 15 17:14:48 2022 +0100

    VERSION: Bump version up to Samba 4.17.4...
    
    and re-enable GIT_SNAPSHOT.
    
    Signed-off-by: Jule Anger <janger at samba.org>
    Signed-off-by: Stefan Metzmacher <metze at samba.org>

commit 120f7790f6ba1bbfaaa117b9e015d9e4632b9f13
Merge: 2803e76fba0 212ebbf7f4f
Author: Jule Anger <janger at samba.org>
Date:   Tue Nov 15 17:13:45 2022 +0100

    Merge tag 'samba-4.17.3' into v4-17-test
    
    samba: tag release samba-4.17.3
    
    Signed-off-by: Jule Anger <janger at samba.org>
    Signed-off-by: Stefan Metzmacher <metze at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 VERSION                                 |   2 +-
 WHATSNEW.txt                            |  50 ++-
 third_party/heimdal/lib/krb5/pac.c      | 614 +++++++++++++++++++++-----------
 third_party/heimdal/lib/krb5/test_pac.c |  48 ++-
 4 files changed, 493 insertions(+), 221 deletions(-)


Changeset truncated at 500 lines:

diff --git a/VERSION b/VERSION
index 59521d4f944..be26434337c 100644
--- a/VERSION
+++ b/VERSION
@@ -25,7 +25,7 @@
 ########################################################
 SAMBA_VERSION_MAJOR=4
 SAMBA_VERSION_MINOR=17
-SAMBA_VERSION_RELEASE=3
+SAMBA_VERSION_RELEASE=4
 
 ########################################################
 # If a official release has a serious bug              #
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index 9b9d644694d..6a9245050ee 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -1,3 +1,50 @@
+                   ==============================
+                   Release Notes for Samba 4.17.3
+                         November 15, 2022
+                   ==============================
+
+
+This is a security release in order to address the following defects:
+
+
+o CVE-2022-42898: Samba's Kerberos libraries and AD DC failed to guard against
+                  integer overflows when parsing a PAC on a 32-bit system, which
+                  allowed an attacker with a forged PAC to corrupt the heap.
+                  https://www.samba.org/samba/security/CVE-2022-42898.html
+
+Changes since 4.17.2
+--------------------
+o  Joseph Sutton <josephsutton at catalyst.net.nz>
+   * BUG 15203: CVE-2022-42898
+
+o  Nicolas Williams <nico at twosigma.com>
+   * BUG 15203: CVE-2022-42898
+
+
+#######################################
+Reporting bugs & Development Discussion
+#######################################
+
+Please discuss this release on the samba-technical mailing list or by
+joining the #samba-technical:matrix.org matrix room, or
+#samba-technical IRC channel on irc.libera.chat.
+
+
+If you do report problems then please try to send high quality
+feedback. If you don't provide vital information to help us track down
+the problem then you will probably be ignored.  All bug reports should
+be filed under the Samba 4.1 and newer product in the project's Bugzilla
+database (https://bugzilla.samba.org/).
+
+
+======================================================================
+== Our Code, Our Bugs, Our Responsibility.
+== The Samba Team
+======================================================================
+
+
+Release notes for older releases follow:
+----------------------------------------
                    ==============================
                    Release Notes for Samba 4.17.2
                           October 25, 2022
@@ -46,8 +93,7 @@ database (https://bugzilla.samba.org/).
 ======================================================================
 
 
-Release notes for older releases follow:
-----------------------------------------
+----------------------------------------------------------------------
                    ==============================
                    Release Notes for Samba 4.17.1
                           October 19, 2022
diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c
index c8f355c8179..c11990a1606 100644
--- a/third_party/heimdal/lib/krb5/pac.c
+++ b/third_party/heimdal/lib/krb5/pac.c
@@ -37,19 +37,34 @@
 #include <wind.h>
 #include <assert.h>
 
+/*
+ * https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-pac/3341cfa2-6ef5-42e0-b7bc-4544884bf399
+ */
 struct PAC_INFO_BUFFER {
-    uint32_t type;
-    uint32_t buffersize;
-    uint32_t offset_hi;
-    uint32_t offset_lo;
+    uint32_t type;          /* ULONG   ulType       in the original */
+    uint32_t buffersize;    /* ULONG   cbBufferSize in the original */
+    uint64_t offset;        /* ULONG64 Offset       in the original
+                             * this being the offset from the beginning of the
+                             * struct PACTYPE to the beginning of the buffer
+                             * containing data of type ulType
+                             */
 };
 
+/*
+ * https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-pac/6655b92f-ab06-490b-845d-037e6987275f
+ */
 struct PACTYPE {
-    uint32_t numbuffers;
-    uint32_t version;
-    struct PAC_INFO_BUFFER buffers[1];
+    uint32_t numbuffers;    /* named cBuffers of type ULONG in the original */
+    uint32_t version;       /* Named Version  of type ULONG in the original */
+    struct PAC_INFO_BUFFER buffers[1]; /* an ellipsis (...) in the original */
 };
 
+/*
+ * A PAC starts with a PACTYPE header structure that is followed by an array of
+ * numbuffers PAC_INFO_BUFFER structures, each of which points to a buffer
+ * beyond the last PAC_INFO_BUFFER structures.
+ */
+
 struct krb5_pac_data {
     struct PACTYPE *pac;
     krb5_data data;
@@ -133,6 +148,60 @@ struct heim_type_data pac_object = {
     NULL
 };
 
+/*
+ * Returns the size of the PACTYPE header + the PAC_INFO_BUFFER array.  This is
+ * also the end of the whole thing, and any offsets to buffers from
+ * thePAC_INFO_BUFFER[] entries have to be beyond it.
+ */
+static krb5_error_code
+pac_header_size(krb5_context context, uint32_t num_buffers, uint32_t *result)
+{
+    krb5_error_code ret;
+    uint32_t header_size;
+
+    /* Guard against integer overflow */
+    if (num_buffers > UINT32_MAX / PAC_INFO_BUFFER_SIZE) {
+	ret = EOVERFLOW;
+	krb5_set_error_message(context, ret, "PAC has too many buffers");
+	return ret;
+    }
+    header_size = PAC_INFO_BUFFER_SIZE * num_buffers;
+
+    /* Guard against integer overflow */
+    if (header_size > UINT32_MAX - PACTYPE_SIZE) {
+	ret = EOVERFLOW;
+	krb5_set_error_message(context, ret, "PAC has too many buffers");
+	return ret;
+    }
+    header_size += PACTYPE_SIZE;
+
+    *result = header_size;
+
+    return 0;
+}
+
+/* Output `size' + `addend' + padding for alignment if it doesn't overflow */
+static krb5_error_code
+pac_aligned_size(krb5_context context,
+                 uint32_t size,
+                 uint32_t addend,
+                 uint32_t *aligned_size)
+{
+    krb5_error_code ret;
+
+    if (size > UINT32_MAX - addend ||
+        (size + addend) > UINT32_MAX - (PAC_ALIGNMENT - 1)) {
+	ret = EOVERFLOW;
+	krb5_set_error_message(context, ret, "integer overrun");
+	return ret;
+    }
+    size += addend;
+    size += PAC_ALIGNMENT - 1;
+    size &= ~(PAC_ALIGNMENT - 1);
+    *aligned_size = size;
+    return 0;
+}
+
 /*
  * HMAC-MD5 checksum over any key (needed for the PAC routines)
  */
@@ -184,165 +253,164 @@ KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
 krb5_pac_parse(krb5_context context, const void *ptr, size_t len,
 	       krb5_pac *pac)
 {
-    krb5_error_code ret;
+    krb5_error_code ret = 0;
     krb5_pac p;
     krb5_storage *sp = NULL;
-    uint32_t i, tmp, tmp2, header_end;
+    uint32_t i, num_buffers, version, header_size = 0;
+    uint32_t prev_start = 0;
+    uint32_t prev_end = 0;
 
+    *pac = NULL;
     p = _heim_alloc_object(&pac_object, sizeof(*p));
-    if (p == NULL) {
-	ret = krb5_enomem(context);
-	goto out;
-    }
-
-    sp = krb5_storage_from_readonly_mem(ptr, len);
-    if (sp == NULL) {
+    if (p)
+        sp = krb5_storage_from_readonly_mem(ptr, len);
+    if (sp == NULL)
 	ret = krb5_enomem(context);
-	goto out;
-    }
-    krb5_storage_set_flags(sp, KRB5_STORAGE_BYTEORDER_LE);
-
-    CHECK(ret, krb5_ret_uint32(sp, &tmp), out);
-    CHECK(ret, krb5_ret_uint32(sp, &tmp2), out);
-    if (tmp < 1) {
-	ret = EINVAL; /* Too few buffers */
-	krb5_set_error_message(context, ret, N_("PAC has too few buffers", ""));
-	goto out;
+    if (ret == 0) {
+        krb5_storage_set_flags(sp, KRB5_STORAGE_BYTEORDER_LE);
+        ret = krb5_ret_uint32(sp, &num_buffers);
     }
-    if (tmp2 != 0) {
-	ret = EINVAL; /* Wrong version */
-	krb5_set_error_message(context, ret,
+    if (ret == 0)
+        ret = krb5_ret_uint32(sp, &version);
+    if (ret == 0 && num_buffers < 1)
+	krb5_set_error_message(context, ret = EINVAL,
+                               N_("PAC has too few buffers", ""));
+    if (ret == 0 && num_buffers > 1000)
+	krb5_set_error_message(context, ret = EINVAL,
+                               N_("PAC has too many buffers", ""));
+    if (ret == 0 && version != 0)
+	krb5_set_error_message(context, ret = EINVAL,
 			       N_("PAC has wrong version %d", ""),
-			       (int)tmp2);
-	goto out;
-    }
-
-    p->pac = calloc(1,
-		    sizeof(*p->pac) + (sizeof(p->pac->buffers[0]) * (tmp - 1)));
-    if (p->pac == NULL) {
+			       (int)version);
+    if (ret == 0)
+        ret = pac_header_size(context, num_buffers, &header_size);
+    if (ret == 0 && header_size > len)
+        krb5_set_error_message(context, ret = EOVERFLOW,
+                               N_("PAC encoding invalid, would overflow buffers", ""));
+    if (ret == 0)
+        p->pac = calloc(1, header_size);
+    if (ret == 0 && p->pac == NULL)
 	ret = krb5_enomem(context);
-	goto out;
-    }
-
-    p->pac->numbuffers = tmp;
-    p->pac->version = tmp2;
 
-    header_end = PACTYPE_SIZE + (PAC_INFO_BUFFER_SIZE * p->pac->numbuffers);
-    if (header_end > len) {
-	ret = EINVAL;
-	goto out;
+    if (ret == 0) {
+        p->pac->numbuffers = num_buffers;
+        p->pac->version = version;
     }
 
-    for (i = 0; i < p->pac->numbuffers; i++) {
-	CHECK(ret, krb5_ret_uint32(sp, &p->pac->buffers[i].type), out);
-	CHECK(ret, krb5_ret_uint32(sp, &p->pac->buffers[i].buffersize), out);
-	CHECK(ret, krb5_ret_uint32(sp, &p->pac->buffers[i].offset_lo), out);
-	CHECK(ret, krb5_ret_uint32(sp, &p->pac->buffers[i].offset_hi), out);
+    for (i = 0; ret == 0 && i < p->pac->numbuffers; i++) {
+        ret = krb5_ret_uint32(sp, &p->pac->buffers[i].type);
+        if (ret == 0)
+            ret = krb5_ret_uint32(sp, &p->pac->buffers[i].buffersize);
+        if (ret == 0)
+            ret = krb5_ret_uint64(sp, &p->pac->buffers[i].offset);
+        if (ret)
+            break;
 
-	/* consistency checks */
-	if (p->pac->buffers[i].offset_lo & (PAC_ALIGNMENT - 1)) {
-	    ret = EINVAL;
-	    krb5_set_error_message(context, ret,
+	/* Consistency checks (we don't check for wasted space) */
+	if (p->pac->buffers[i].offset & (PAC_ALIGNMENT - 1)) {
+	    krb5_set_error_message(context, ret = EINVAL,
 				   N_("PAC out of alignment", ""));
-	    goto out;
+	    break;
 	}
-	if (p->pac->buffers[i].offset_hi) {
-	    ret = EINVAL;
-	    krb5_set_error_message(context, ret,
-				   N_("PAC high offset set", ""));
-	    goto out;
+	if (p->pac->buffers[i].offset > len ||
+            p->pac->buffers[i].buffersize > len ||
+            len - p->pac->buffers[i].offset < p->pac->buffers[i].buffersize) {
+	    krb5_set_error_message(context, ret = EOVERFLOW,
+				   N_("PAC buffer overflow", ""));
+	    break;
 	}
-	if (p->pac->buffers[i].offset_lo > len) {
-	    ret = EINVAL;
-	    krb5_set_error_message(context, ret,
-				   N_("PAC offset overflow", ""));
-	    goto out;
-	}
-	if (p->pac->buffers[i].offset_lo < header_end) {
-	    ret = EINVAL;
-	    krb5_set_error_message(context, ret,
+	if (p->pac->buffers[i].offset < header_size) {
+	    krb5_set_error_message(context, ret = EINVAL,
 				   N_("PAC offset inside header: %lu %lu", ""),
-				   (unsigned long)p->pac->buffers[i].offset_lo,
-				   (unsigned long)header_end);
-	    goto out;
-	}
-	if (p->pac->buffers[i].buffersize > len - p->pac->buffers[i].offset_lo){
-	    ret = EINVAL;
-	    krb5_set_error_message(context, ret, N_("PAC length overflow", ""));
-	    goto out;
+				   (unsigned long)p->pac->buffers[i].offset,
+				   (unsigned long)header_size);
+	    break;
 	}
 
-	/* let save pointer to data we need later */
-	if (p->pac->buffers[i].type == PAC_SERVER_CHECKSUM) {
-	    if (p->server_checksum) {
-		ret = EINVAL;
-		krb5_set_error_message(context, ret,
+        /*
+         * We'd like to check for non-overlapping of buffers, but the buffers
+         * need not be in the same order as the PAC_INFO_BUFFER[] entries
+         * pointing to them!  To fully check for overlap we'd have to have an
+         * O(N^2) loop after we parse all the PAC_INFO_BUFFER[].
+         *
+         * But we can check that each buffer does not overlap the previous
+         * buffer.
+         */
+        if (prev_start) {
+            if (p->pac->buffers[i].offset >= prev_start &&
+                p->pac->buffers[i].offset <  prev_end) {
+                krb5_set_error_message(context, ret = EINVAL,
+                                       N_("PAC overlap", ""));
+                break;
+            }
+            if (p->pac->buffers[i].offset < prev_start &&
+                p->pac->buffers[i].offset +
+                p->pac->buffers[i].buffersize > prev_start) {
+                krb5_set_error_message(context, ret = EINVAL,
+                                       N_("PAC overlap", ""));
+                break;
+            }
+        }
+        prev_start = p->pac->buffers[i].offset;
+        prev_end = p->pac->buffers[i].offset + p->pac->buffers[i].buffersize;
+
+	/* Let's save pointers to buffers we'll need later */
+        switch (p->pac->buffers[i].type) {
+        case PAC_SERVER_CHECKSUM:
+	    if (p->server_checksum)
+		krb5_set_error_message(context, ret = EINVAL,
 				       N_("PAC has multiple server checksums", ""));
-		goto out;
-	    }
-	    p->server_checksum = &p->pac->buffers[i];
-	} else if (p->pac->buffers[i].type == PAC_PRIVSVR_CHECKSUM) {
-	    if (p->privsvr_checksum) {
-		ret = EINVAL;
-		krb5_set_error_message(context, ret,
-				       N_("PAC has multiple KDC checksums", ""));
-		goto out;
-	    }
-	    p->privsvr_checksum = &p->pac->buffers[i];
-	} else if (p->pac->buffers[i].type == PAC_LOGON_NAME) {
-	    if (p->logon_name) {
-		ret = EINVAL;
-		krb5_set_error_message(context, ret,
-				       N_("PAC has multiple logon names", ""));
-		goto out;
-	    }
-	    p->logon_name = &p->pac->buffers[i];
-	} else if (p->pac->buffers[i].type == PAC_UPN_DNS_INFO) {
-	    if (p->upn_dns_info) {
-		ret = EINVAL;
-		krb5_set_error_message(context, ret,
+	    else
+                p->server_checksum = &p->pac->buffers[i];
+            break;
+        case PAC_PRIVSVR_CHECKSUM:
+	    if (p->privsvr_checksum)
+                krb5_set_error_message(context, ret = EINVAL,
+                                       N_("PAC has multiple KDC checksums", ""));
+            else
+                p->privsvr_checksum = &p->pac->buffers[i];
+            break;
+        case PAC_LOGON_NAME:
+	    if (p->logon_name)
+		krb5_set_error_message(context, ret = EINVAL,
+                                       N_("PAC has multiple logon names", ""));
+            else
+                p->logon_name = &p->pac->buffers[i];
+            break;
+        case PAC_UPN_DNS_INFO:
+	    if (p->upn_dns_info)
+		krb5_set_error_message(context, ret = EINVAL,
 				       N_("PAC has multiple UPN DNS info buffers", ""));
-		goto out;
-	    }
-	    p->upn_dns_info = &p->pac->buffers[i];
-	} else if (p->pac->buffers[i].type == PAC_TICKET_CHECKSUM) {
-	    if (p->ticket_checksum) {
-		ret = EINVAL;
-		krb5_set_error_message(context, ret,
+            else
+                p->upn_dns_info = &p->pac->buffers[i];
+            break;
+        case PAC_TICKET_CHECKSUM:
+	    if (p->ticket_checksum)
+		krb5_set_error_message(context, ret = EINVAL,
 				       N_("PAC has multiple ticket checksums", ""));
-		goto out;
-	    }
-	    p->ticket_checksum = &p->pac->buffers[i];
-	} else if (p->pac->buffers[i].type == PAC_ATTRIBUTES_INFO) {
-	    if (p->attributes_info) {
-		ret = EINVAL;
-		krb5_set_error_message(context, ret,
+            else
+                p->ticket_checksum = &p->pac->buffers[i];
+            break;
+        case PAC_ATTRIBUTES_INFO:
+	    if (p->attributes_info)
+		krb5_set_error_message(context, ret = EINVAL,
 				       N_("PAC has multiple attributes info buffers", ""));
-		goto out;
-	    }
-	    p->attributes_info = &p->pac->buffers[i];
-	}
+            else
+                p->attributes_info = &p->pac->buffers[i];
+            break;
+        default: break;
+        }
     }
 
-    ret = krb5_data_copy(&p->data, ptr, len);
-    if (ret)
-	goto out;
-
-    krb5_storage_free(sp);
-
-    *pac = p;
-    return 0;
-
-out:
-    if (sp)
-	krb5_storage_free(sp);
-    if (p) {
-	if (p->pac)
-	    free(p->pac);
-	krb5_pac_free(context, p);
+    if (ret == 0)
+        ret = krb5_data_copy(&p->data, ptr, len);
+    if (ret == 0) {
+        *pac = p;
+        p = NULL;
     }
-    *pac = NULL;
-
+    if (sp)
+        krb5_storage_free(sp);
+    krb5_pac_free(context, p);
     return ret;
 }
 
@@ -369,77 +437,111 @@ krb5_pac_init(krb5_context context, krb5_pac *pac)
 	krb5_pac_free(context, p);
 	return krb5_enomem(context);
     }
+    memset(p->data.data, 0, p->data.length);
 
     *pac = p;
     return 0;
 }
 
+/**
+ * Add a PAC buffer `nd' of type `type' to the pac `p'.
+ *
+ * @param context
+ * @param p
+ * @param type


-- 
Samba Shared Repository



More information about the samba-cvs mailing list