bpo-28503: Use crypt_r() when available instead of crypt() (GH-11373) (GH-11376)

Use crypt_r() when available instead of crypt() in the crypt module.

As a nice side effect: This also avoids a memory sanitizer flake as clang msan doesn't know about crypt's internal libc allocated buffer.
(cherry picked from commit 387512c7ecde6446f2e29408af2e16b9fc043807)

Co-authored-by: Gregory P. Smith <greg@krypto.org> [Google]

diff --git a/Include/Python.h b/Include/Python.h
index 05113735c156443a6b169bdcf08b883623c67dee..54ea3214810e1b1cb731740f3d3795f222cf70c0 100644 (file)
--- a/Include/Python.h
+++ b/Include/Python.h
@@ -36,7 +36,17 @@
 #include <unistd.h>
 #endif
 #ifdef HAVE_CRYPT_H
+#if defined(HAVE_CRYPT_R) && !defined(_GNU_SOURCE)
+/* Required for glibc to expose the crypt_r() function prototype. */
+#  define _GNU_SOURCE
+#  define _Py_GNU_SOURCE_FOR_CRYPT
+#endif
 #include <crypt.h>
+#ifdef _Py_GNU_SOURCE_FOR_CRYPT
+/* Don't leak the _GNU_SOURCE define to other headers. */
+#  undef _GNU_SOURCE
+#  undef _Py_GNU_SOURCE_FOR_CRYPT
+#endif
 #endif
 
 /* For size_t? */

diff --git a/Misc/NEWS.d/next/Library/2018-12-30-14-56-33.bpo-28503.V4kNN3.rst b/Misc/NEWS.d/next/Library/2018-12-30-14-56-33.bpo-28503.V4kNN3.rst
new file mode 100644 (file)
index 0000000..651fef1
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-12-30-14-56-33.bpo-28503.V4kNN3.rst
@@ -0,0 +1,2 @@
+The `crypt` module now internally uses the `crypt_r()` library function
+instead of `crypt()` when available.

diff --git a/Modules/_cryptmodule.c b/Modules/_cryptmodule.c
index 58d179e6a3d2d15b3f4db5f098ba2af732e11fe0..5d03f45f643615315ddaf20758c0dcd659e6a6db 100644 (file)
--- a/Modules/_cryptmodule.c
+++ b/Modules/_cryptmodule.c
@@ -34,7 +34,15 @@ static PyObject *
 crypt_crypt_impl(PyObject *module, const char *word, const char *salt)
 /*[clinic end generated code: output=0512284a03d2803c input=0e8edec9c364352b]*/
 {
-    return Py_BuildValue("s", crypt(word, salt));
+    char *crypt_result;
+#ifdef HAVE_CRYPT_R
+    struct crypt_data data;
+    memset(&data, 0, sizeof(data));
+    crypt_result = crypt_r(word, salt, &data);
+#else
+    crypt_result = crypt(word, salt);
+#endif
+    return Py_BuildValue("s", crypt_result);
 }
 
 

diff --git a/configure b/configure
index 5e5f974b0e5064b0b0425e7cac651bb4fafdf83c..7afd48966f61284f705d580022bf03d6c78bd58d 100755 (executable)
--- a/configure
+++ b/configure
@@ -12727,6 +12727,150 @@ fi
 done
 
 
+# We search for both crypt and crypt_r as one or the other may be defined
+# This gets us our -lcrypt in LIBS when required on the target platform.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing crypt" >&5
+$as_echo_n "checking for library containing crypt... " >&6; }
+if ${ac_cv_search_crypt+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_func_search_save_LIBS=$LIBS
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char crypt ();
+int
+main ()
+{
+return crypt ();
+  ;
+  return 0;
+}
+_ACEOF
+for ac_lib in '' crypt; do
+  if test -z "$ac_lib"; then
+    ac_res="none required"
+  else
+    ac_res=-l$ac_lib
+    LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
+  fi
+  if ac_fn_c_try_link "$LINENO"; then :
+  ac_cv_search_crypt=$ac_res
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext
+  if ${ac_cv_search_crypt+:} false; then :
+  break
+fi
+done
+if ${ac_cv_search_crypt+:} false; then :
+
+else
+  ac_cv_search_crypt=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_crypt" >&5
+$as_echo "$ac_cv_search_crypt" >&6; }
+ac_res=$ac_cv_search_crypt
+if test "$ac_res" != no; then :
+  test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing crypt_r" >&5
+$as_echo_n "checking for library containing crypt_r... " >&6; }
+if ${ac_cv_search_crypt_r+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_func_search_save_LIBS=$LIBS
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char crypt_r ();
+int
+main ()
+{
+return crypt_r ();
+  ;
+  return 0;
+}
+_ACEOF
+for ac_lib in '' crypt; do
+  if test -z "$ac_lib"; then
+    ac_res="none required"
+  else
+    ac_res=-l$ac_lib
+    LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
+  fi
+  if ac_fn_c_try_link "$LINENO"; then :
+  ac_cv_search_crypt_r=$ac_res
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext
+  if ${ac_cv_search_crypt_r+:} false; then :
+  break
+fi
+done
+if ${ac_cv_search_crypt_r+:} false; then :
+
+else
+  ac_cv_search_crypt_r=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_crypt_r" >&5
+$as_echo "$ac_cv_search_crypt_r" >&6; }
+ac_res=$ac_cv_search_crypt_r
+if test "$ac_res" != no; then :
+  test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+fi
+
+
+ac_fn_c_check_func "$LINENO" "crypt_r" "ac_cv_func_crypt_r"
+if test "x$ac_cv_func_crypt_r" = xyes; then :
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+#define _GNU_SOURCE  /* Required for crypt_r()'s prototype in glibc. */
+#include <crypt.h>
+
+int
+main ()
+{
+
+struct crypt_data d;
+char *r = crypt_r("", "", &d);
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+$as_echo "#define HAVE_CRYPT_R 1" >>confdefs.h
+
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
+
+
 for ac_func in clock_gettime
 do :
   ac_fn_c_check_func "$LINENO" "clock_gettime" "ac_cv_func_clock_gettime"

diff --git a/configure.ac b/configure.ac
index a7de901e08b7864b7f5824b4ad4b4030ba98d819..788c361ffeeb26814eac811ef55b9dfb619736ff 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -3875,6 +3875,23 @@ AC_CHECK_FUNCS(gettimeofday,
     ])
 )
 
+# We search for both crypt and crypt_r as one or the other may be defined
+# This gets us our -lcrypt in LIBS when required on the target platform.
+AC_SEARCH_LIBS(crypt, crypt)
+AC_SEARCH_LIBS(crypt_r, crypt)
+
+AC_CHECK_FUNC(crypt_r,
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+#define _GNU_SOURCE  /* Required for crypt_r()'s prototype in glibc. */
+#include <crypt.h>
+]], [[
+struct crypt_data d;
+char *r = crypt_r("", "", &d);
+]])],
+    [AC_DEFINE(HAVE_CRYPT_R, 1, [Define if you have the crypt_r() function.])],
+    [])
+)
+
 AC_CHECK_FUNCS(clock_gettime, [], [
     AC_CHECK_LIB(rt, clock_gettime, [
         LIBS="$LIBS -lrt"

diff --git a/pyconfig.h.in b/pyconfig.h.in
index a82e82cc9cccacd6a6366d472ab4be936da5fd59..4032fa519f8abb05ccc8c52b204e43baee73766a 100644 (file)
--- a/pyconfig.h.in
+++ b/pyconfig.h.in
@@ -143,6 +143,9 @@
 /* Define to 1 if you have the <crypt.h> header file. */
 #undef HAVE_CRYPT_H
 
+/* Define if you have the crypt_r() function. */
+#undef HAVE_CRYPT_R
+
 /* Define to 1 if you have the `ctermid' function. */
 #undef HAVE_CTERMID