crypt: Get rid of alloca usage in __md5_crypt_r

Message ID 20230915141302.2093730-1-josimmon@redhat.com
State Superseded
Headers
Series crypt: Get rid of alloca usage in __md5_crypt_r |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed

Commit Message

Joe Simmons-Talbott Sept. 15, 2023, 2:12 p.m. UTC
  Replace alloca usage with scratch_buffers to avoid potential stack
overflow.
---
 crypt/md5-crypt.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)
  

Patch

diff --git a/crypt/md5-crypt.c b/crypt/md5-crypt.c
index 4e4760545a..891bfedf88 100644
--- a/crypt/md5-crypt.c
+++ b/crypt/md5-crypt.c
@@ -19,6 +19,7 @@ 
 
 #include <assert.h>
 #include <errno.h>
+#include <scratch_buffer.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/param.h>
@@ -99,7 +100,10 @@  __md5_crypt_r (const char *key, const char *salt, char *buffer, int buflen)
   char *copied_key = NULL;
   char *copied_salt = NULL;
   char *free_key = NULL;
-  size_t alloca_used = 0;
+  struct scratch_buffer sbuf;
+  scratch_buffer_init (&sbuf);
+  struct scratch_buffer saltbuf;
+  scratch_buffer_init (&saltfuf);
 
   /* Find beginning of salt string.  The prefix should normally always
      be present.  Just in case it is not.  */
@@ -114,14 +118,11 @@  __md5_crypt_r (const char *key, const char *salt, char *buffer, int buflen)
     {
       char *tmp;
 
-      if (__libc_use_alloca (alloca_used + key_len + __alignof__ (md5_uint32)))
-	tmp = (char *) alloca (key_len + __alignof__ (md5_uint32));
-      else
-	{
-	  free_key = tmp = (char *) malloc (key_len + __alignof__ (md5_uint32));
-	  if (tmp == NULL)
-	    return NULL;
-	}
+      if (!scratch_buffer_set_array_size (
+		&sbuf, 1, key_len + __alignof__ (md5_unint32))
+        return NULL;
+
+      tmp = sbuf.data;
 
       key = copied_key =
 	memcpy (tmp + __alignof__ (md5_uint32)
@@ -132,7 +133,15 @@  __md5_crypt_r (const char *key, const char *salt, char *buffer, int buflen)
 
   if (((uintptr_t) salt) % __alignof__ (md5_uint32) != 0)
     {
-      char *tmp = (char *) alloca (salt_len + __alignof__ (md5_uint32));
+      if (!scratch_buffer_set_array_size (
+		&saltbuf, salt_len * __alignof__ (md5_uint32)))
+        {
+          scratch_buffer_free (&sbuf);
+          return NULL;
+	}
+
+      char *tmp = saltbuf.data;
+
       salt = copied_salt =
 	memcpy (tmp + __alignof__ (md5_uint32)
 		- ((uintptr_t) tmp) % __alignof__ (md5_uint32),
@@ -145,7 +154,8 @@  __md5_crypt_r (const char *key, const char *salt, char *buffer, int buflen)
   NSSLOWInitContext *nss_ictx = NSSLOW_Init ();
   if (nss_ictx == NULL)
     {
-      free (free_key);
+      scratch_buffer_free (&sbuf);
+      scratch_buffer_free (&saltbuf);
       return NULL;
     }
   NSSLOWHASHContext *nss_ctx = NULL;
@@ -295,7 +305,8 @@  __md5_crypt_r (const char *key, const char *salt, char *buffer, int buflen)
   if (copied_salt != NULL)
     explicit_bzero (copied_salt, salt_len);
 
-  free (free_key);
+  scratch_buffer_free (&sbuf);
+  scratch_buffer_free (&saltbuf);
   return buffer;
 }