Skip to contentSkip to conversation

Fix issue with newer glibc crypt() function

PATCH New Saul Wold 1 participant 3 messages Apr 5, 2013

lore
Share & tools
lore

Saul Wold

author
#0 lore raw

cover Fix issue with newer glibc crypt() function

The 2.17 version of glibc's crypt() was changed to returning NULL if it got an invalid seed, this broke tinylogin and shadow in different ways.

Thanks to the Slackware team for "just in time patches", they found it first in shadow around March 28th!

I do not believe that these will introduce any security concerns, please review with a careful eye.

Sau!

Saul Wold (2):
tinylogin: fix segfault from crypt()
shadow: add patch to fix crypt: Invalid Argument

 .../tinylogin/tinylogin-1.4/glibc_crypt_fix.patch  | 23 ++++++++
 meta/recipes-core/tinylogin/tinylogin_1.4.bb       |  4 +-
 .../files/slackware_fix_for_glib-2.17_crypt.patch  | 63 ++++++++++++++++++++++
 meta/recipes-extended/shadow/shadow_4.1.4.3.bb     |  1 +
 4 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-core/tinylogin/tinylogin-1.4/glibc_crypt_fix.patch
 create mode 100644 meta/recipes-extended/shadow/files/slackware_fix_for_glib-2.17_crypt.patch
Signature · 1 line
1.8.0.2
author
#1 lore raw

PATCH 1/2 tinylogin: fix segfault from crypt()

In glibc 2.17, crypt() now expects 2 valid chars for the seed or it will error out and return a NULL. The tinylogin code took the result from crypt directly into a strcmp() which caused a segfault

Tinylogin has been deperacted, busybox now has login support, I will investigate using busybox login support for 1.5.

[YOCTO #4097]

---
 .../tinylogin/tinylogin-1.4/glibc_crypt_fix.patch  | 23 ++++++++++++++++++++++
 meta/recipes-core/tinylogin/tinylogin_1.4.bb       |  4 +++-
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-core/tinylogin/tinylogin-1.4/glibc_crypt_fix.patch
meta/recipes-core/tinylogin/tinylogin-1.4/glibc_crypt_fix.patch new file +23 -0
new file mode 100644
index 0000000..0a24656
@@ -0,0 +1,23 @@
+
+staring from glibc 2.17 the crypt() function will error out and return
+NULL if the seed or "correct" is invalid. The failure case for this is
+an unknown user which tinylogin assigns '!' for the password. crypt() 
+now expects a minimum of 2 valid characters. If we get a NULL return
+value from the crypt, assume we fail and return 0.
+
+Upstream-Status: Inappropriate [tinylogin depercated]
+Signed-off-by: Saul Wold <[email protected]>
+
+Index: tinylogin-1.4/libbb/correct_password.c
+===================================================================
+--- tinylogin-1.4.orig/libbb/correct_password.c
++++ tinylogin-1.4/libbb/correct_password.c
+@@ -74,5 +74,8 @@ int correct_password ( const struct pass
+ 	}
+ 	encrypted = crypt ( unencrypted, correct );
+ 	memset ( unencrypted, 0, xstrlen ( unencrypted ));
++        if ( !encrypted )
++		return 0;
++
+ 	return ( strcmp ( encrypted, correct ) == 0 ) ? 1 : 0;
+ }
meta/recipes-core/tinylogin/tinylogin_1.4.bb +3 -1
index aaed92a..6e08011 100644
@@ -18,7 +18,9 @@ SRC_URI = "http://www.angstrom-distribution.org/unstable/sources/tinylogin-${PV}
 	file://remove-index.patch \
 	file://use_O2_option.patch \
 	file://passwd_rotate_check.patch \
-	file://avoid_static.patch"
+	file://avoid_static.patch \
+        file://glibc_crypt_fix.patch \
+        "
 
 SRC_URI[md5sum] = "44da0ff2b727455669890b24305e351d"
 SRC_URI[sha256sum] = "5e542e4b7825305a3678bf73136c392feb0d44b8bbf926e8eda5453eea7ddd6b"
Signature · 1 line
1.8.0.2
author
#2 lore raw

PATCH 2/2 shadow: add patch to fix crypt: Invalid Argument

In reply to Saul Wold

This patch came from Slackware and address a change in crypt()'s handling of an invalid seed, which in the past returned an encrypted string and now returns a NULL.

[YOCTO #4097] related to tinylogin segfault

---
 .../files/slackware_fix_for_glib-2.17_crypt.patch  | 63 ++++++++++++++++++++++
 meta/recipes-extended/shadow/shadow_4.1.4.3.bb     |  1 +
 2 files changed, 64 insertions(+)
 create mode 100644 meta/recipes-extended/shadow/files/slackware_fix_for_glib-2.17_crypt.patch
meta/recipes-extended/shadow/files/slackware_fix_for_glib-2.17_crypt.patch new file +63 -0
new file mode 100644
index 0000000..7cd45af
@@ -0,0 +1,63 @@
+
+This patch is from Slackware, I tried to find the actual
+author to add that attribution.  The comment below is the
+best summary, I will not repeat it here.
+
+Upstream-Status: Backport from slackware
+
+Signed-off-by: Saul Wold <[email protected]>
+
+Index: shadow-4.1.4.3/lib/encrypt.c
+===================================================================
+--- shadow-4.1.4.3.orig/lib/encrypt.c
++++ shadow-4.1.4.3/lib/encrypt.c
+@@ -45,15 +45,40 @@ char *pw_encrypt (const char *clear, con
+ 	static char cipher[128];
+ 	char *cp;
+ 
+-	cp = crypt (clear, salt);
+-	if (!cp) {
+-		/*
+-		 * Single Unix Spec: crypt() may return a null pointer,
+-		 * and set errno to indicate an error.  The caller doesn't
+-		 * expect us to return NULL, so...
+-		 */
+-		perror ("crypt");
+-		exit (EXIT_FAILURE);
++ 	cp = crypt (clear, salt);
++ 	if (!cp) {
++ 		/*
++		 * In glibc-2.17 and newer, crypt() will return NULL if
++		 * it was called using an invalid salt format.  Previous
++		 * versions of glibc would go ahead and compute a DES hash
++		 * using the invalid salt.  The salt value in this case was
++		 * always '!'.  We might arrive at this place if either the
++		 * user does not exist, or if the hash in /etc/shadow doesn't
++		 * have the proper magic for one of the supported hash
++		 * formats (for example, if the account was locked using
++		 * "passwd -l".  To handle this situation, we will recompute
++		 * the hash using a hardcoded salt as was previously done
++		 * by glibc.  The hash returned by the old glibc function
++		 * always began with "!!", which would ensure that it could
++		 * never match an otherwise valid hash in /etc/shadow that
++		 * was disabled with a "!" at the beginning (since the second
++		 * character would never be "!" as well), so we will also
++		 * prepend the resulting hash with "!!".  Finally, in case
++		 * crypt() failed for some other reason we will check to see
++		 * if we still get NULL from crypt even with the valid salt
++		 * and will fail if that's the case.
++ 		 */
++
++		/* Recalculate hash using a hardcoded, valid SHA512 salt: */
++		cp = crypt (clear, "$6$8IIcy/1EPOk/");
++
++		if (!cp) {
++			perror ("crypt");
++			exit (EXIT_FAILURE);
++		} else {
++			sprintf (cipher, "!!%s", cp);
++			return cipher;
++		}
+ 	}
+ 
+ 	/* The GNU crypt does not return NULL if the algorithm is not
meta/recipes-extended/shadow/shadow_4.1.4.3.bb +1 -0
index 8226a6e..caf24c0 100644
@@ -23,6 +23,7 @@ SRC_URI = "http://pkg-shadow.alioth.debian.org/releases/${BPN}-${PV}.tar.bz2 \
            file://shadow-4.1.4.2-su_no_sanitize_env.patch \
            file://shadow-update-pam-conf.patch \
            file://shadow_fix_for_automake-1.12.patch \
+           file://slackware_fix_for_glib-2.17_crypt.patch \
            "
 
 SRC_URI[md5sum] = "b8608d8294ac88974f27b20f991c0e79"
Signature · 1 line
1.8.0.2