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 387512c)

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

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? */

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.

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);
 }
 
 

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"

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"

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