From: Greg Hudson <ghudson@mit.edu>
Date: Wed, 8 Feb 2023 17:23:28 +0000 (-0500)
Subject: Fix read overruns in SPNEGO parsing
X-Git-Tag: krb5-1.21-beta1~27
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=47c2a12830dbd7fb8e13c239ddc0ac74129a91f6;p=thirdparty%2Fkrb5.git

Fix read overruns in SPNEGO parsing

Fix three read overruns discovered by the GitHub Security Lab team
(GHSL-2023-016, GHSL-2023-017, and GHSL-2023-018) using OSS-Fuzz.

In get_mech_set(), error out if gss_add_oid_set_member() fails rather
than continue the loop and increment i past the current bound of
returned_mechSet.  In g_verify_neg_token_init(), check for zero-byte
sequences before reading tag bytes, and reduce cur_size by one to
account for the tag byte when calling gssint_get_der_length().

ticket: 9085 (new)
tags: pullup
target_version: 1.20-next
target_version: 1.19-next
---

diff --git a/src/lib/gssapi/spnego/spnego_mech.c b/src/lib/gssapi/spnego/spnego_mech.c
index ba7765cb41..654964c628 100644
--- a/src/lib/gssapi/spnego/spnego_mech.c
+++ b/src/lib/gssapi/spnego/spnego_mech.c
@@ -3455,7 +3455,7 @@ get_mech_set(OM_uint32 *minor_status, unsigned char **buff_in,
 	     unsigned int buff_length)
 {
 	gss_OID_set returned_mechSet;
-	OM_uint32 major_status;
+	OM_uint32 major_status, tmpmin;
 	int length;
 	unsigned int bytes;
 	OM_uint32 set_length;
@@ -3485,9 +3485,12 @@ get_mech_set(OM_uint32 *minor_status, unsigned char **buff_in,
 
 		major_status = gss_add_oid_set_member(minor_status,
 						      temp, &returned_mechSet);
-		if (major_status == GSS_S_COMPLETE)
-			set_length += returned_mechSet->elements[i].length +2;
 		generic_gss_release_oid(minor_status, &temp);
+		if (major_status != GSS_S_COMPLETE) {
+			gss_release_oid_set(&tmpmin, &returned_mechSet);
+			return (NULL);
+		}
+		set_length += returned_mechSet->elements[i].length + 2;
 	}
 
 	return (returned_mechSet);
@@ -4305,7 +4308,8 @@ g_verify_neg_token_init(unsigned char **buf_in, unsigned int cur_size)
 	 * - check for a0(context specific identifier)
 	 * - get length and verify that enoughd ata exists
 	 */
-	if (g_get_tag_and_length(&buf, CONTEXT, cur_size, &bytes) < 0)
+	if (g_get_tag_and_length(&buf, CONTEXT, cur_size, &bytes) < 0 ||
+	    bytes == 0)
 		return (G_BAD_TOK_HEADER);
 
 	cur_size = bytes; /* should indicate bytes remaining */
@@ -4315,7 +4319,7 @@ g_verify_neg_token_init(unsigned char **buf_in, unsigned int cur_size)
 	 * a strucure of type NegTokenInit.
 	 */
 	if (*buf++ == SEQUENCE) {
-		if ((seqsize = gssint_get_der_length(&buf, cur_size, &bytes)) < 0)
+		if ((seqsize = gssint_get_der_length(&buf, cur_size - 1, &bytes)) <= 0)
 			return (G_BAD_TOK_HEADER);
 		/*
 		 * Make sure we have the entire buffer as described
@@ -4332,7 +4336,7 @@ g_verify_neg_token_init(unsigned char **buf_in, unsigned int cur_size)
 	 * Verify that the first blob is a sequence of mechTypes
 	 */
 	if (*buf++ == CONTEXT) {
-		if ((seqsize = gssint_get_der_length(&buf, cur_size, &bytes)) < 0)
+		if ((seqsize = gssint_get_der_length(&buf, cur_size - 1, &bytes)) < 0)
 			return (G_BAD_TOK_HEADER);
 		/*
 		 * Make sure we have the entire buffer as described