Fix incorrect password transformation in contrib/pgcrypto's DES crypt().

tglsfdc · tglsfdc · commit 932ded2ed51e · 2012-05-30T10:53:30.000-04:00
Overly tight coding caused the password transformation loop to stop examining input once it had processed a byte equal to 0x80. Thus, if the given password string contained such a byte (which is possible though not highly likely in UTF8, and perhaps also in other non-ASCII encodings), all subsequent characters would not contribute to the hash, making the password much weaker than it appears on the surface. This would only affect cases where applications used DES crypt() to encode passwords before storing them in the database. If a weak password has been created in this fashion, the hash will stop matching after this update has been applied, so it will be easy to tell if any passwords were unexpectedly weak. Changing to a different password would be a good idea in such a case. (Since DES has been considered inadequately secure for some time, changing to a different encryption algorithm can also be recommended.) This code, and the bug, are shared with at least PHP, FreeBSD, and OpenBSD. Since the other projects have already published their fixes, there is no point in trying to keep this commit private. This bug has been assigned CVE-2012-2143, and credit for its discovery goes to Rubin Xu and Joseph Bonneau.
diff --git a/contrib/pgcrypto/crypt-des.c b/contrib/pgcrypto/crypt-des.c
@@ -670,7 +670,8 @@ px_crypt_des(const char *key, const char *setting)
 	q = (uint8 *) keybuf;
 	while (q - (uint8 *) keybuf - 8)
 	{
-		if ((*q++ = *key << 1))
+		*q++ = *key << 1;
+		if (*key != '\0')
 			key++;
 	}
 	if (des_setkey((char *) keybuf))

Original file line number	Diff line number	Diff line change
`@@ -670,7 +670,8 @@ px_crypt_des(const char key, const char setting)`
`670`	`670`	`q = (uint8 *) keybuf;`
`671`	`671`	`while (q - (uint8 *) keybuf - 8)`
`672`	`672`	`{`
`673`		`- if ((q++ = key << 1))`
	`673`	`+ q++ = key << 1;`
	`674`	`+ if (*key != '\0')`
`674`	`675`	`key++;`
`675`	`676`	`}`
`676`	`677`	`if (des_setkey((char *) keybuf))`