Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
8d45443bb5c9 pppd: Ignore received EAP messages when not doing EAP 8d7970b8f3db pppd: Fix bounds check in EAP code 858976b1fc31 radius: Prevent buffer overflow in rc_mksid() Signed-off-by: Petr Štetiar <ynezz@true.cz> (cherry picked from commit 215598f) Fixes: CVE-2020-8597 Signed-off-by: Jo-Philipp Wich <jo@mein.io>
- Loading branch information
Showing
4 changed files
with
129 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
30 changes: 30 additions & 0 deletions
30
package/network/services/ppp/patches/700-radius-Prevent-buffer-overflow-in-rc_mksid.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
From 858976b1fc3107f1261aae337831959b511b83c2 Mon Sep 17 00:00:00 2001 | ||
From: Paul Mackerras <paulus@ozlabs.org> | ||
Date: Sat, 4 Jan 2020 12:01:32 +1100 | ||
Subject: [PATCH] radius: Prevent buffer overflow in rc_mksid() | ||
|
||
On some systems getpid() can return a value greater than 65535. | ||
Increase the size of buf[] to allow for this, and use slprintf() | ||
to make sure we never overflow it. | ||
|
||
Signed-off-by: Paul Mackerras <paulus@ozlabs.org> | ||
--- | ||
pppd/plugins/radius/util.c | 4 ++-- | ||
1 file changed, 2 insertions(+), 2 deletions(-) | ||
|
||
diff --git a/pppd/plugins/radius/util.c b/pppd/plugins/radius/util.c | ||
index 6f976a712951..740131e8377c 100644 | ||
--- a/pppd/plugins/radius/util.c | ||
+++ b/pppd/plugins/radius/util.c | ||
@@ -73,9 +73,9 @@ void rc_mdelay(int msecs) | ||
char * | ||
rc_mksid (void) | ||
{ | ||
- static char buf[15]; | ||
+ static char buf[32]; | ||
static unsigned short int cnt = 0; | ||
- sprintf (buf, "%08lX%04X%02hX", | ||
+ slprintf(buf, sizeof(buf), "%08lX%04X%02hX", | ||
(unsigned long int) time (NULL), | ||
(unsigned int) getpid (), | ||
cnt & 0xFF); |
37 changes: 37 additions & 0 deletions
37
package/network/services/ppp/patches/701-pppd-Fix-bounds-check-in-EAP-code.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
From 8d7970b8f3db727fe798b65f3377fe6787575426 Mon Sep 17 00:00:00 2001 | ||
From: Paul Mackerras <paulus@ozlabs.org> | ||
Date: Mon, 3 Feb 2020 15:53:28 +1100 | ||
Subject: [PATCH] pppd: Fix bounds check in EAP code | ||
|
||
Given that we have just checked vallen < len, it can never be the case | ||
that vallen >= len + sizeof(rhostname). This fixes the check so we | ||
actually avoid overflowing the rhostname array. | ||
|
||
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> | ||
Signed-off-by: Paul Mackerras <paulus@ozlabs.org> | ||
--- | ||
pppd/eap.c | 4 ++-- | ||
1 file changed, 2 insertions(+), 2 deletions(-) | ||
|
||
diff --git a/pppd/eap.c b/pppd/eap.c | ||
index 94407f56a336..1b93db01aebd 100644 | ||
--- a/pppd/eap.c | ||
+++ b/pppd/eap.c | ||
@@ -1420,7 +1420,7 @@ int len; | ||
} | ||
|
||
/* Not so likely to happen. */ | ||
- if (vallen >= len + sizeof (rhostname)) { | ||
+ if (len - vallen >= sizeof (rhostname)) { | ||
dbglog("EAP: trimming really long peer name down"); | ||
BCOPY(inp + vallen, rhostname, sizeof (rhostname) - 1); | ||
rhostname[sizeof (rhostname) - 1] = '\0'; | ||
@@ -1846,7 +1846,7 @@ int len; | ||
} | ||
|
||
/* Not so likely to happen. */ | ||
- if (vallen >= len + sizeof (rhostname)) { | ||
+ if (len - vallen >= sizeof (rhostname)) { | ||
dbglog("EAP: trimming really long peer name down"); | ||
BCOPY(inp + vallen, rhostname, sizeof (rhostname) - 1); | ||
rhostname[sizeof (rhostname) - 1] = '\0'; |
61 changes: 61 additions & 0 deletions
61
...twork/services/ppp/patches/702-pppd-Ignore-received-EAP-messages-when-not-doing-EAP.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
From 8d45443bb5c9372b4c6a362ba2f443d41c5636af Mon Sep 17 00:00:00 2001 | ||
From: Paul Mackerras <paulus@ozlabs.org> | ||
Date: Mon, 3 Feb 2020 16:31:42 +1100 | ||
Subject: [PATCH] pppd: Ignore received EAP messages when not doing EAP | ||
|
||
This adds some basic checks to the subroutines of eap_input to check | ||
that we have requested or agreed to doing EAP authentication before | ||
doing any processing on the received packet. The motivation is to | ||
make it harder for a malicious peer to disrupt the operation of pppd | ||
by sending unsolicited EAP packets. Note that eap_success() already | ||
has a check that the EAP client state is reasonable, and does nothing | ||
(apart from possibly printing a debug message) if not. | ||
|
||
Signed-off-by: Paul Mackerras <paulus@ozlabs.org> | ||
--- | ||
pppd/eap.c | 18 ++++++++++++++++++ | ||
1 file changed, 18 insertions(+) | ||
|
||
diff --git a/pppd/eap.c b/pppd/eap.c | ||
index 1b93db01aebd..082e95343120 100644 | ||
--- a/pppd/eap.c | ||
+++ b/pppd/eap.c | ||
@@ -1328,6 +1328,12 @@ int len; | ||
int fd; | ||
#endif /* USE_SRP */ | ||
|
||
+ /* | ||
+ * Ignore requests if we're not open | ||
+ */ | ||
+ if (esp->es_client.ea_state <= eapClosed) | ||
+ return; | ||
+ | ||
/* | ||
* Note: we update es_client.ea_id *only if* a Response | ||
* message is being generated. Otherwise, we leave it the | ||
@@ -1736,6 +1742,12 @@ int len; | ||
u_char dig[SHA_DIGESTSIZE]; | ||
#endif /* USE_SRP */ | ||
|
||
+ /* | ||
+ * Ignore responses if we're not open | ||
+ */ | ||
+ if (esp->es_server.ea_state <= eapClosed) | ||
+ return; | ||
+ | ||
if (esp->es_server.ea_id != id) { | ||
dbglog("EAP: discarding Response %d; expected ID %d", id, | ||
esp->es_server.ea_id); | ||
@@ -2047,6 +2059,12 @@ u_char *inp; | ||
int id; | ||
int len; | ||
{ | ||
+ /* | ||
+ * Ignore failure messages if we're not open | ||
+ */ | ||
+ if (esp->es_client.ea_state <= eapClosed) | ||
+ return; | ||
+ | ||
if (!eap_client_active(esp)) { | ||
dbglog("EAP unexpected failure message in state %s (%d)", | ||
eap_state_name(esp->es_client.ea_state), |