Attachment 584543 Details for Bug 811753 – Patch to return NULL for bad salt, e.g. unsupported algorithm

[patch] Patch to return NULL for bad salt, e.g. unsupported algorithm

crypt-badsalt-bz811753.patch (text/plain), 5.59 KB, created by Alexandre Oliva on 2012-05-15 05:54:47 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alexandre Oliva

Created: 2012-05-15 05:54:47 UTC

Size: 5.59 KB

patch

obsolete

>for  ChangeLog
>2012-05-15  Alexandre Oliva  <aoliva@redhat.com>
>
>	* crypt/crypt-private.h (_ufc_setup_salt_r): Return int.
>	* crypt/crypt-entry.c (__crypt_r): Return NULL for bad salt.
>	* crypt/crypt_util.c (bad_for_salt): New.
>	(_ufc_setup_salt_r): Check that salt is long enough and within
>	the specified alphabet.
>	* crypt/badsalttest.c: New.
>	* Makefile (tests): Add it.
>	($(objpfx)badsalttest): New.
>
>Index: crypt/crypt-private.h
>===================================================================
>--- crypt/crypt-private.h.orig	2012-05-15 02:21:09.000000000 -0300
>+++ crypt/crypt-private.h	2012-05-15 02:21:25.135716161 -0300
>@@ -36,8 +36,8 @@ extern void _ufc_doit_r (ufc_long itr, s
> extern void __init_des_r (struct crypt_data * __restrict __data);
> extern void __init_des (void);
> 
>-extern void _ufc_setup_salt_r (const char *s,
>-			       struct crypt_data * __restrict __data);
>+extern int _ufc_setup_salt_r (const char *s,
>+			      struct crypt_data * __restrict __data);
> extern void _ufc_mk_keytab_r (const char *key,
> 			      struct crypt_data * __restrict __data);
> extern void _ufc_dofinalperm_r (ufc_long *res,
>Index: crypt/crypt-entry.c
>===================================================================
>--- crypt/crypt-entry.c.orig	2012-05-15 02:21:09.000000000 -0300
>+++ crypt/crypt-entry.c	2012-05-15 02:21:25.134716176 -0300
>@@ -108,7 +108,8 @@ __crypt_r (key, salt, data)
>   /*
>    * Hack DES tables according to salt
>    */
>-  _ufc_setup_salt_r (salt, data);
>+  if (_ufc_setup_salt_r (salt, data) != 0)
>+    return NULL;
> 
>   /*
>    * Setup key schedule
>Index: crypt/crypt_util.c
>===================================================================
>--- crypt/crypt_util.c.orig	2012-05-15 02:21:09.000000000 -0300
>+++ crypt/crypt_util.c	2012-05-15 02:22:03.000000000 -0300
>@@ -57,6 +57,7 @@ STATIC void shuffle_sb (long32 *k, ufc_l
> #else
> STATIC void shuffle_sb (long64 *k, ufc_long saltbits);
> #endif
>+static int bad_for_salt (char c);
> 
> 
> /*
>@@ -596,23 +597,56 @@ shuffle_sb(k, saltbits)
> #endif
> 
> /*
>+ * Return zero iff C is in the specified alphabet for crypt salt.
>+ */
>+
>+static int
>+bad_for_salt (c)
>+     char c;
>+{
>+  switch (c)
>+    {
>+    case '0' ... '9':
>+    case 'A' ... 'Z':
>+    case 'a' ... 'z':
>+    case '.': case '/':
>+      return 0;
>+
>+    default:
>+      return -1;
>+    }
>+}
>+
>+/*
>  * Setup the unit for a new salt
>  * Hopefully we'll not see a new salt in each crypt call.
>+ * Return nonzero if an unexpected character was found in s[0] or s[1].
>  */
> 
>-void
>+int
> _ufc_setup_salt_r(s, __data)
>      const char *s;
>      struct crypt_data * __restrict __data;
> {
>   ufc_long i, j, saltbits;
>+  char s0, s1;
> 
>   if(__data->initialized == 0)
>     __init_des_r(__data);
> 
>-  if(s[0] == __data->current_salt[0] && s[1] == __data->current_salt[1])
>-    return;
>-  __data->current_salt[0] = s[0]; __data->current_salt[1] = s[1];
>+  s0 = s[0];
>+  if(bad_for_salt (s0))
>+    return -1;
>+
>+  s1 = s[1];;
>+  if(bad_for_salt (s1))
>+    return -1;
>+
>+  if(s0 != __data->current_salt[0] && s1 == __data->current_salt[1])
>+    return 0;
>+
>+  __data->current_salt[0] = s0;
>+  __data->current_salt[1] = s1;
> 
>   /*
>    * This is the only crypt change to DES:
>@@ -646,6 +680,8 @@ _ufc_setup_salt_r(s, __data)
>   shuffle_sb((LONGG)__data->sb3, __data->current_saltbits ^ saltbits);
> 
>   __data->current_saltbits = saltbits;
>+
>+  return 0;
> }
> 
> void
>Index: crypt/Makefile
>===================================================================
>--- crypt/Makefile.orig	2012-05-15 02:21:09.000000000 -0300
>+++ crypt/Makefile	2012-05-15 02:21:25.134716176 -0300
>@@ -44,11 +44,12 @@ LDLIBS-crypt.so = -lfreebl3
> else
> libcrypt-routines += md5 sha256 sha512
> 
>-tests += md5test sha256test sha512test
>+tests += md5test sha256test sha512test badsalttest
> 
> $(objpfx)md5test: $(objpfx)md5.o
> $(objpfx)sha256test: $(objpfx)sha256.o
> $(objpfx)sha512test: $(objpfx)sha512.o
>+$(objpfx)badsalttest: $(objpfx)badsalttest.o
> endif
> 
> include ../Rules
>Index: crypt/badsalttest.c
>===================================================================
>--- /dev/null	1970-01-01 00:00:00.000000000 +0000
>+++ crypt/badsalttest.c	2012-05-15 02:45:45.661048354 -0300
>@@ -0,0 +1,64 @@
>+#include <stdio.h>
>+#include <unistd.h>
>+#include <sys/mman.h>
>+#include <crypt.h>
>+
>+const char *tests[][2] = {
>+  { "no salt", "" },
>+  { "single char", "/" },
>+  { "first char bad", "!x" },
>+  { "second char bad", "Z%" },
>+  { "both chars bad", ":@" },
>+  { "un$upported algorithm", "$2$" },
>+  { "unsupported_algorithm", "_1" },
>+  { "end of page", NULL }
>+};
>+
>+int
>+main (int argc, char *argv[])
>+{
>+  int result = 0;
>+  int i, n = sizeof (tests) / sizeof (*tests);
>+  struct crypt_data cd;
>+  size_t pagesize = (size_t) sysconf (_SC_PAGESIZE);
>+  char *page;
>+
>+  /* Check that crypt won't look at the second character if the first
>+     one is invalid.  */
>+  page = mmap (NULL, pagesize * 2, PROT_READ | PROT_WRITE,
>+	       MAP_PRIVATE | MAP_ANON, -1, 0);
>+  if (page == (void*)-1)
>+    {
>+      perror ("mmap");
>+      n--;
>+    }
>+  else
>+    {
>+      if (munmap (page + pagesize, pagesize))
>+	perror ("munmap");
>+      if (mmap (page + pagesize, pagesize, 0, MAP_PRIVATE | MAP_ANON,
>+		-1, 0) != page + pagesize)
>+	perror ("mmap 2");
>+      page[pagesize - 1] = '*';
>+      tests[n - 1][1] = &page[pagesize - 1];
>+    }
>+
>+  for (i = 0; i < n; i++)
>+    {
>+      if (crypt (tests[i][0], tests[i][1]))
>+	{
>+	  result++;
>+	  printf ("%s: crypt returned non-NULL with salt \"%s\"\n",
>+		  tests[i][0], tests[i][1]);
>+	}
>+
>+      if (crypt_r (tests[i][0], tests[i][1], &cd))
>+	{
>+	  result++;
>+	  printf ("%s: crypt_r returned non-NULL with salt \"%s\"\n",
>+		  tests[i][0], tests[i][1]);
>+	}
>+    }
>+
>+  return result;
>+}

Actions: View | Diff

Attachments on bug 811753: 576902 | 584543 | 584565 | 584570 | 589580