[net.bugs] Bug in crypt.c; the DES algorithm

sob@wjh12.UUCP (Bradner) (11/02/83)

   There is a problem in the standard /usr/src/lib/libc/crypt.c file
loaded as part of everyone's libc. The bug has yet to be fixed even in
4.2, so everyone (V7, 2.X, 4.X) probably has it except perhaps system
III or V (but check! I was amazed to find that no one caught this bug
even though it has probably been in libc since V7 !!).

   When crypt(3) was added to /usr/src/lib/libc/crypt.c to permute the
existing DES algorithm (and thereby get "safer" password encryptions),
a bug was introduced in the normal calling sequence for setkey(3) and
encrypt(3) which implement the standard DES algorithm.

   The problem arises because the new routine crypt() tries to modify
DES by changing the array E[] from its initial value by first copying in
the e[] array and then changing various entries. Meanwhile, anyone not
calling crypt() but trying to use the normal setkey()/encrypt() DES will
have an empty E[] array (or worse, a mangled one if they called crypt()
first) since setkey() was not modified to copy e[] to E[] after crypt()
was added (this is an example of a common software maintenance problem).
Crypt(3) itself worked correctly and is not affected by this bug (nor by
the fix given below). Password encryption works but DES alone does not.

   The fix is to have setkey() copy the initial E[] array from the e[]
array; however, they E[] and e[] arrays are declared after setkey() and
so they must be moved to before its body to allow the change (Note: this
shows another reason why no one thought to fix E[] in setkey(), it was
not even "aware" of the existence of the E[] array!).

   The following is a diff of the crypt.c changes I added for the 4.1c/4.2
version of the problem; you can ignore them if you simply (1) move the 
declaration of E[] and e[] to before setkey() and add as the last two lines
of the setkey routine the for loop you see below (that simply copies the
elements of e[] into E[]). That this will not affect crypt(3) because it
permutes the E[] array after it calls setkey().

diff crypt.c crypt.old.c
------------------------
95,109d94
<  * The E bit-selection table.
<  */
< static	char	E[48];		/* Moved by clp; 10/6/83 (see below) */
< static	char	e[] = {
< 	32, 1, 2, 3, 4, 5,
< 	 4, 5, 6, 7, 8, 9,
< 	 8, 9,10,11,12,13,
< 	12,13,14,15,16,17,
< 	16,17,18,19,20,21,
< 	20,21,22,23,24,25,
< 	24,25,26,27,28,29,
< 	28,29,30,31,32, 1,
< };
< 
< /*
156,157d140
< 	for(i=0;i<48;i++)	/*  Added by clp; 10/6/83 */
< 		E[i] = e[i];	/* (these must be set up) */
158a142,156
> 
> /*
>  * The E bit-selection table.
>  */
> static	char	E[48];
> static	char	e[] = {
> 	32, 1, 2, 3, 4, 5,
> 	 4, 5, 6, 7, 8, 9,
> 	 8, 9,10,11,12,13,
> 	12,13,14,15,16,17,
> 	16,17,18,19,20,21,
> 	20,21,22,23,24,25,
> 	24,25,26,27,28,29,
> 	28,29,30,31,32, 1,
> };
-------------------------------------------------------------------------

   Anyone in BELL should take note to be sure system III, etc. have fixed
this problem since DES may be useful in business applications (apparently
it hasn't been used much elsewhere, or others would have noticed this bug
earlier).
							Charles L. Perkins
						    ...ucbvax!ucbernie!clp

-- 

	scott bradner - harvard university

uucp:		...{genrad,allegra,ihnp4}!wjh12!sob
bitnet:		sob @ harvunxw