diff mbox series

CVE-2018-11574 version range fix

Message ID 20230903000100.0c1b187b@windsurf
State Not Applicable
Headers show
Series CVE-2018-11574 version range fix | expand

Commit Message

Thomas Petazzoni Sept. 2, 2023, 10:01 p.m. UTC 
Hello,

CVE-2018-11574 is marked in the NVD database as affecting all pppd
versions, as it has as its only "Configuration" the following CPE
identifier match:

  cpe:2.3:a:point-to-point_protocol_project:point-to-point_protocol:-:*:*:*:*:*:*:*

However, it turns out that the upstream pppd was *never* affected by
CVE-2018-11574. Let me walk through the story.

CVE-2018-11574 affects the EAP-TLS implementation in pppd. However,
EAP-TLS was not supported in upstream pppd before its 2.4.9 release,
thanks to commit
https://github.com/ppp-project/ppp/commit/e87fe1bbd37a1486c5223f110e9ce3ef75971f93.

Before that EAP-TLS support for pppd was provided as an out-of-tree
patch, provided by a third party developer at
https://jjkeijser.github.io/ppp/download.html. It is this patch that
was affected by CVE-2018-11574. As can be seen at
https://jjkeijser.github.io/ppp/download.html, all versions of the
patch prior to version 1.101 are affected, as 1.101 was precisely
released to fix CVE-2018-11574.

So: before pppd 2.4.9, the only way to be affected by CVE-2018-11574
was by having applied a third-party patch. I am not sure how to reflect
this correctly in the CVE-2018-11574 information in the NVD database.
To me, if one applies random patches to a code base, for sure those
patches can introduce additional security vulnerabilities, so it
doesn't make sense that CVE-2018-11574 is reported against pppd
upstream.

In addition, in the EAP-TLS code that was added in pppd 2.4.9, the
issue of CVE-2018-11574 is already fixed. Indeed, we did a diff between
the out-of-tree EAP-TLS patch in version 0.999 (affected) and 1.101
(not affected), which gives the attached file. And those fixes are
indeed present in commit
https://github.com/ppp-project/ppp/commit/e87fe1bbd37a1486c5223f110e9ce3ef75971f93,
which introduced EAP-TLS support in upstream pppd.

Therefore: upstream pppd was never affected by this issue. Prior to
pppd 2.4.9, there was no EAP-TLS support, and starting from 2.4.9, the
EAP-TLS is correct with regard to CVE-2018-11574.

At the very least, I would suggest to change the CVE-2018-11574
information to indicate that only versions up to (and excluding) 2.4.9
are affected. Do you think this would be possible ?

Best regards,

Thomas

Comments

Thomas Petazzoni Sept. 8, 2023, 8:59 p.m. UTC | #1 
Hello,

Gentle ping on the below bug report. Thanks!

Thomas Petazzoni

On Sun, 3 Sep 2023 00:01:00 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> Hello,
> 
> CVE-2018-11574 is marked in the NVD database as affecting all pppd
> versions, as it has as its only "Configuration" the following CPE
> identifier match:
> 
>   cpe:2.3:a:point-to-point_protocol_project:point-to-point_protocol:-:*:*:*:*:*:*:*
> 
> However, it turns out that the upstream pppd was *never* affected by
> CVE-2018-11574. Let me walk through the story.
> 
> CVE-2018-11574 affects the EAP-TLS implementation in pppd. However,
> EAP-TLS was not supported in upstream pppd before its 2.4.9 release,
> thanks to commit
> https://github.com/ppp-project/ppp/commit/e87fe1bbd37a1486c5223f110e9ce3ef75971f93.
> 
> Before that EAP-TLS support for pppd was provided as an out-of-tree
> patch, provided by a third party developer at
> https://jjkeijser.github.io/ppp/download.html. It is this patch that
> was affected by CVE-2018-11574. As can be seen at
> https://jjkeijser.github.io/ppp/download.html, all versions of the
> patch prior to version 1.101 are affected, as 1.101 was precisely
> released to fix CVE-2018-11574.
> 
> So: before pppd 2.4.9, the only way to be affected by CVE-2018-11574
> was by having applied a third-party patch. I am not sure how to reflect
> this correctly in the CVE-2018-11574 information in the NVD database.
> To me, if one applies random patches to a code base, for sure those
> patches can introduce additional security vulnerabilities, so it
> doesn't make sense that CVE-2018-11574 is reported against pppd
> upstream.
> 
> In addition, in the EAP-TLS code that was added in pppd 2.4.9, the
> issue of CVE-2018-11574 is already fixed. Indeed, we did a diff between
> the out-of-tree EAP-TLS patch in version 0.999 (affected) and 1.101
> (not affected), which gives the attached file. And those fixes are
> indeed present in commit
> https://github.com/ppp-project/ppp/commit/e87fe1bbd37a1486c5223f110e9ce3ef75971f93,
> which introduced EAP-TLS support in upstream pppd.
> 
> Therefore: upstream pppd was never affected by this issue. Prior to
> pppd 2.4.9, there was no EAP-TLS support, and starting from 2.4.9, the
> EAP-TLS is correct with regard to CVE-2018-11574.
> 
> At the very least, I would suggest to change the CVE-2018-11574
> information to indicate that only versions up to (and excluding) 2.4.9
> are affected. Do you think this would be possible ?
> 
> Best regards,
> 
> Thomas
diff mbox series

Patch

diff --git a/README.eap-tls b/README.eap-tls
index 037be0a..084b046 100644
--- a/README.eap-tls
+++ b/README.eap-tls
@@ -273,8 +273,14 @@  v0.995   (27-May-2014)
 v0.996   (28-May-2014)
  - Fix minor bug where SessionTicket message was printed as 'Unknown SSL3 code 4'
  - Add EAP-TLS-specific options to pppd.8 manual page.
- - Updated README.eap-tls file with new option and provide an example.
+ - Updated README.eap-tls file with new options and provide an example.
 v0.997   (19-Jun-2014)
- - change SSL_OP_NO_TICKETS to SSL_OP_NO_TICKET
- - fix bug in initialisation code with fragmented packets.
+ - Change SSL_OP_NO_TICKETS to SSL_OP_NO_TICKET
+ - Fix bug in initialisation code with fragmented packets.
+v0.998   (13-Mar-2015)
+ - Added fix for https://bugzilla.redhat.com/show_bug.cgi?id=1023620
+v0.999   (11-May-2017)
+ - Added support for OpenSSL 1.1: the code will now compile against OpenSSL 1.0.x or 1.1.x.
+v1.101 (1-Jun-2018)
+ - Fix vulnerabilities CVE-2018-11574.
 
diff --git a/pppd/eap-tls.c b/pppd/eap-tls.c
index 2c0766e..1b79abf 100644
--- a/pppd/eap-tls.c
+++ b/pppd/eap-tls.c
@@ -62,6 +62,7 @@  static ENGINE *pkey_engine = NULL;
  * tries to provide some guidance but ultimately falls short.
  */
 
+
 static void HMAC_CTX_free(HMAC_CTX *ctx)
 {
 	if (ctx != NULL) {
@@ -865,46 +866,47 @@  void eaptls_free_session(struct eaptls_session *ets)
 int eaptls_receive(struct eaptls_session *ets, u_char * inp, int len)
 {
 	u_char flags;
-	u_int tlslen;
+	u_int tlslen = 0;
 	u_char dummy[65536];
 
+	if (len < 1) {
+		warn("EAP-TLS: received no or invalid data");
+		return 1;
+	}
+		
 	GETCHAR(flags, inp);
 	len--;
 
-    if (flags & EAP_TLS_FLAGS_LI && !ets->data) {
- 
+    if (flags & EAP_TLS_FLAGS_LI && len >= 4) {
 		/*
-		 * This is the first packet of a message
+		 * LenghtIncluded flag set -> this is the first packet of a message
 		*/
- 
+
+		/*
+		 * the first 4 octets are the length of the EAP-TLS message
+		 */
 		GETLONG(tlslen, inp);
 		len -= 4;
 
-		if (tlslen > EAP_TLS_MAX_LEN) {
-			error("Error: tls message length > %d, truncated",
-				EAP_TLS_MAX_LEN);
-			tlslen = EAP_TLS_MAX_LEN;
-		}
+		if (!ets->data) {
 
-		/*
-		 * Allocate memory for the whole message
-		*/
-		ets->data = malloc(tlslen);
-		if (!ets->data)
-			fatal("EAP TLS: allocation error\n");
+			if (tlslen > EAP_TLS_MAX_LEN) {
+				error("EAP-TLS: TLS message length > %d, truncated", EAP_TLS_MAX_LEN);
+				tlslen = EAP_TLS_MAX_LEN;
+			}
 
-		ets->datalen = 0;
-		ets->tlslen = tlslen;
+			/*
+			 * Allocate memory for the whole message
+			*/
+			ets->data = malloc(tlslen);
+			if (!ets->data)
+				fatal("EAP-TLS: allocation error\n");
 
-	}
-	else if (flags & EAP_TLS_FLAGS_LI && ets->data) {
-		/*
-		 * Non first with LI (strange...)
-		*/
- 
-		GETLONG(tlslen, inp);
-		len -= 4;
- 
+			ets->datalen = 0;
+			ets->tlslen = tlslen;
+		}
+		else
+			warn("EAP-TLS: non-first LI packet? that's odd...");
 	}
 	else if (!ets->data) {
 		/*
@@ -913,7 +915,7 @@  int eaptls_receive(struct eaptls_session *ets, u_char * inp, int len)
  
 		ets->data = malloc(len);
 		if (!ets->data)
-			fatal("EAP TLS: allocation error\n");
+			fatal("EAP-TLS: allocation error\n");
  
 		ets->datalen = 0;
 		ets->tlslen = len;
@@ -924,8 +926,13 @@  int eaptls_receive(struct eaptls_session *ets, u_char * inp, int len)
 	else
 		ets->frag = 0;
 
+	if (len < 0) {
+		warn("EAP-TLS: received malformed data");
+		return 1;
+	}
+
 	if (len + ets->datalen > ets->tlslen) {
-		warn("EAP TLS: received data > TLS message length");
+		warn("EAP-TLS: received data > TLS message length");
 		return 1;
 	}
 
@@ -939,7 +946,7 @@  int eaptls_receive(struct eaptls_session *ets, u_char * inp, int len)
 		 */
 
 		if (ets->datalen != ets->tlslen) {
-			warn("EAP TLS: received data != TLS message length");
+			warn("EAP-TLS: received data != TLS message length");
 			return 1;
 		}
 
@@ -1200,7 +1207,6 @@  ssl_msg_callback(int write_p, int version, int content_type,
 
 	switch(content_type) {
 
-#if OPENSSL_VERSION_NUMBER >= 0x10100000L
 	case SSL3_RT_HEADER:
 		strcat(string, "SSL/TLS Header: ");
 		switch(hvers) {
@@ -1226,7 +1232,6 @@  ssl_msg_callback(int write_p, int version, int content_type,
 			strcat(string, "Unknown version");
 		}
 		break;
-#endif /* OPENSSL_VERSION_NUMBER >= 0x10100000L */
 
 	case SSL3_RT_ALERT:	
 		strcat(string, "Alert: ");	
diff --git a/pppd/eap.c b/pppd/eap.c
index 008801b..3ce8bde 100644
--- a/pppd/eap.c
+++ b/pppd/eap.c
@@ -1703,6 +1703,11 @@  int len;
 
 		case eapListen:
 
+			if (len < 1) {
+				error("EAP: received EAP-TLS Listen packet with no data");
+				/* Bogus request; wait for something real. */
+				return;
+			}
 			GETCHAR(flags, inp);
 			if(flags & EAP_TLS_FLAGS_START){
 
@@ -1740,6 +1745,11 @@  int len;
 			break;
 
 		case eapTlsRecv:
+			if (len < 1) {
+				error("EAP: discarding EAP-TLS Receive packet with no data");
+				/* Bogus request; wait for something real. */
+				return;
+			}
 			eaptls_receive(ets, inp, len);
 
 			if(ets->frag) {
@@ -2110,6 +2120,7 @@  int len;
 		switch(esp->es_server.ea_state) {
 
 		case eapTlsRecv:
+	
 			ets = (struct eaptls_session *) esp->es_server.ea_session;
 			eap_figure_next_state(esp, 
 				eaptls_receive(esp->es_server.ea_session, inp, len));
@@ -2130,19 +2141,22 @@  int len;
 		case eapTlsRecvClient:
 			/* Receive authentication response from client */
 
-			GETCHAR(flags, inp);
+			if (len > 0) {
+				GETCHAR(flags, inp);
 
-			if(len == 1 && !flags) {	/* Ack = ok */
+				if(len == 1 && !flags) {	/* Ack = ok */
 #ifdef MPPE
- 				eaptls_gen_mppe_keys( esp->es_server.ea_session, "client EAP encryption", 0 );
+ 					eaptls_gen_mppe_keys( esp->es_server.ea_session, "client EAP encryption", 0 );
 #endif
-				eap_send_success(esp);
-			}
-			else {			/* failure */
-				eaptls_receive(esp->es_server.ea_session, inp, len);
-				warn("Server authentication failed");
-				eap_send_failure(esp);
+					eap_send_success(esp);
+				}
+				else {			/* failure */
+					warn("Server authentication failed");
+					eap_send_failure(esp);
+				}
 			}
+			else
+				warn("Bogus EAP-TLS packet received from client");
 
 			eaptls_free_session(esp->es_server.ea_session);