Change 575701 by justing@justing_ns1_spectrabsd on 2012/07/13 15:57:57

	Make the PAM password strength checking module WARNS=2 safe.
	
	lib/libpam/modules/pam_passwdqc/Makefile:
		Bump WARNS to 2.
	
	contrib/pam_modules/pam_passwdqc/pam_passwdqc.c:
		Bump  _XOPEN_SOURCE and _XOPEN_VERSION from 500 to 600
		so that vsnprint() is declared.
	
		Use the two new union types (pam_conv_item_t and
		pam_text_item_t) to resolve strict aliasing violations
		caused by casts to comply with the pam_get_item() API taking
		a "const void **" for all item types.
	
		Correct a CLANG "printf-like function with more arguments
		than format" warning.

Affected files ...

... //SpectraBSD/stable/contrib/pam_modules/pam_passwdqc/pam_passwdqc.c#2 edit
... //SpectraBSD/stable/lib/libpam/modules/pam_passwdqc/Makefile#2 edit

Differences ...

==== //SpectraBSD/stable/contrib/pam_modules/pam_passwdqc/pam_passwdqc.c#2 (text) ====

@@ -2,9 +2,9 @@
  * Copyright (c) 2000-2002 by Solar Designer. See LICENSE.
  */
 
-#define _XOPEN_SOURCE 500
+#define _XOPEN_SOURCE 600
 #define _XOPEN_SOURCE_EXTENDED
-#define _XOPEN_VERSION 500
+#define _XOPEN_VERSION 600
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>
@@ -38,7 +38,16 @@
 #else
 #define lo_const			const
 #endif
-typedef lo_const void *pam_item_t;
+
+typedef union {
+	struct pam_conv	*conv;
+	lo_const void	*item;
+} pam_conv_item_t;
+
+typedef union {
+	char		*text;
+	lo_const void	*item;
+} pam_text_item_t;
 
 #include "passwdqc.h"
 
@@ -132,21 +141,21 @@
 static int converse(pam_handle_t *pamh, int style, lo_const char *text,
     struct pam_response **resp)
 {
-	struct pam_conv *conv;
+	pam_conv_item_t conv;
 	struct pam_message msg, *pmsg;
 	int status;
 
-	status = pam_get_item(pamh, PAM_CONV, (pam_item_t *)&conv);
+	status = pam_get_item(pamh, PAM_CONV, &conv.item);
 	if (status != PAM_SUCCESS)
 		return status;
 
 	pmsg = &msg;
 	msg.msg_style = style;
-	msg.msg = text;
+	msg.msg = (char *)text;
 
 	*resp = NULL;
-	return conv->conv(1, (lo_const struct pam_message **)&pmsg, resp,
-	    conv->appdata_ptr);
+	return conv.conv->conv(1, (lo_const struct pam_message **)&pmsg, resp,
+	    conv.conv->appdata_ptr);
 }
 
 #ifdef __GNUC__
@@ -294,8 +303,11 @@
 	}
 
 	if (argc) {
-		say(pamh, PAM_ERROR_MSG, getuid() != 0 ?
-		    MESSAGE_MISCONFIGURED : MESSAGE_INVALID_OPTION, *argv);
+		if (getuid() != 0) {
+			say(pamh, PAM_ERROR_MSG, MESSAGE_MISCONFIGURED);
+		} else {
+			say(pamh, PAM_ERROR_MSG, MESSAGE_INVALID_OPTION, *argv);
+		}
 		return PAM_ABORT;
 	}
 
@@ -311,7 +323,7 @@
 #ifdef HAVE_SHADOW
 	struct spwd *spw;
 #endif
-	char *user, *oldpass, *newpass, *randompass;
+	pam_text_item_t user, oldpass, newpass, randompass;
 	const char *reason;
 	int ask_oldauthtok;
 	int randomonly, enforce, retries_left, retry_wanted;
@@ -353,34 +365,34 @@
 	if (flags & PAM_PRELIM_CHECK)
 		return status;
 
-	status = pam_get_item(pamh, PAM_USER, (pam_item_t *)&user);
+	status = pam_get_item(pamh, PAM_USER, &user.item);
 	if (status != PAM_SUCCESS)
 		return status;
 
-	status = pam_get_item(pamh, PAM_OLDAUTHTOK, (pam_item_t *)&oldpass);
+	status = pam_get_item(pamh, PAM_OLDAUTHTOK, &oldpass.item);
 	if (status != PAM_SUCCESS)
 		return status;
 
 	if (params.flags & F_NON_UNIX) {
 		pw = &fake_pw;
-		pw->pw_name = user;
+		pw->pw_name = user.text;
 		pw->pw_gecos = "";
 	} else {
-		pw = getpwnam(user);
+		pw = getpwnam(user.text);
 		endpwent();
 		if (!pw)
 			return PAM_USER_UNKNOWN;
 		if ((params.flags & F_CHECK_OLDAUTHTOK) && getuid() != 0) {
-			if (!oldpass)
+			if (!oldpass.text)
 				status = PAM_AUTH_ERR;
 			else
 #ifdef HAVE_SHADOW
 			if (!strcmp(pw->pw_passwd, "x")) {
-				spw = getspnam(user);
+				spw = getspnam(user.text);
 				endspent();
 				if (spw) {
-					if (strcmp(crypt(oldpass, spw->sp_pwdp),
-					    spw->sp_pwdp))
+					if (strcmp(crypt(oldpass.text,
+					    spw->sp_pwdp), spw->sp_pwdp))
 						status = PAM_AUTH_ERR;
 					memset(spw->sp_pwdp, 0,
 					    strlen(spw->sp_pwdp));
@@ -388,7 +400,7 @@
 					status = PAM_AUTH_ERR;
 			} else
 #endif
-			if (strcmp(crypt(oldpass, pw->pw_passwd),
+			if (strcmp(crypt(oldpass.text, pw->pw_passwd),
 			    pw->pw_passwd))
 				status = PAM_AUTH_ERR;
 		}
@@ -405,13 +417,14 @@
 		enforce = params.flags & F_ENFORCE_ROOT;
 
 	if (params.flags & F_USE_AUTHTOK) {
-		status = pam_get_item(pamh, PAM_AUTHTOK,
-		    (pam_item_t *)&newpass);
+		status = pam_get_item(pamh, PAM_AUTHTOK, &newpass.item);
 		if (status != PAM_SUCCESS)
 			return status;
-		if (!newpass || (check_max(&params, pamh, newpass) && enforce))
+		if (!newpass.text ||
+		    (check_max(&params, pamh, newpass.text) && enforce))
 			return PAM_AUTHTOK_ERR;
-		reason = _passwdqc_check(&params.qc, newpass, oldpass, pw);
+		reason = _passwdqc_check(&params.qc, newpass.text,
+		    oldpass.text, pw);
 		if (reason) {
 			say(pamh, PAM_ERROR_MSG, MESSAGE_WEAKPASS, reason);
 			if (enforce)
@@ -457,13 +470,13 @@
 			return status;
 	}
 
-	randompass = _passwdqc_random(&params.qc);
-	if (randompass) {
+	randompass.text = _passwdqc_random(&params.qc);
+	if (randompass.text) {
 		status = say(pamh, PAM_TEXT_INFO, randomonly ?
-		    MESSAGE_RANDOMONLY : MESSAGE_RANDOM, randompass);
+		    MESSAGE_RANDOMONLY : MESSAGE_RANDOM, randompass.text);
 		if (status != PAM_SUCCESS) {
-			_pam_overwrite(randompass);
-			randompass = NULL;
+			_pam_overwrite(randompass.text);
+			randompass.text = NULL;
 		}
 	} else
 	if (randomonly) {
@@ -477,29 +490,30 @@
 		status = PAM_AUTHTOK_ERR;
 
 	if (status != PAM_SUCCESS) {
-		if (randompass) _pam_overwrite(randompass);
+		if (randompass.text) _pam_overwrite(randompass.text);
 		return status;
 	}
 
-	newpass = strdup(resp->resp);
+	newpass.text = strdup(resp->resp);
 
 	_pam_drop_reply(resp, 1);
 
-	if (!newpass) {
-		if (randompass) _pam_overwrite(randompass);
+	if (!newpass.text) {
+		if (randompass.text) _pam_overwrite(randompass.text);
 		return PAM_AUTHTOK_ERR;
 	}
 
-	if (check_max(&params, pamh, newpass) && enforce) {
+	if (check_max(&params, pamh, newpass.text) && enforce) {
 		status = PAM_AUTHTOK_ERR;
 		retry_wanted = 1;
 	}
 
 	reason = NULL;
 	if (status == PAM_SUCCESS &&
-	    (!randompass || !strstr(newpass, randompass)) &&
+	    (!randompass.text || !strstr(newpass.text, randompass.text)) &&
 	    (randomonly ||
-	    (reason = _passwdqc_check(&params.qc, newpass, oldpass, pw)))) {
+	    (reason = _passwdqc_check(&params.qc, newpass.text,
+	    oldpass.text, pw)))) {
 		if (randomonly)
 			say(pamh, PAM_ERROR_MSG, MESSAGE_NOTRANDOM);
 		else
@@ -515,7 +529,7 @@
 		    PROMPT_NEWPASS2, &resp);
 	if (status == PAM_SUCCESS) {
 		if (resp && resp->resp) {
-			if (strcmp(newpass, resp->resp)) {
+			if (strcmp(newpass.text, resp->resp)) {
 				status = say(pamh,
 				    PAM_ERROR_MSG, MESSAGE_MISTYPED);
 				if (status == PAM_SUCCESS) {
@@ -529,11 +543,11 @@
 	}
 
 	if (status == PAM_SUCCESS)
-		status = pam_set_item(pamh, PAM_AUTHTOK, newpass);
+		status = pam_set_item(pamh, PAM_AUTHTOK, newpass.text);
 
-	if (randompass) _pam_overwrite(randompass);
-	_pam_overwrite(newpass);
-	free(newpass);
+	if (randompass.text) _pam_overwrite(randompass.text);
+	_pam_overwrite(newpass.text);
+	free(newpass.text);
 
 	if (retry_wanted && --retries_left > 0) {
 		status = say(pamh, PAM_TEXT_INFO, MESSAGE_RETRY);

==== //SpectraBSD/stable/lib/libpam/modules/pam_passwdqc/Makefile#2 (text) ====

@@ -7,7 +7,7 @@
 SRCS=	pam_passwdqc.c passwdqc_check.c passwdqc_random.c wordset_4k.c
 MAN=	pam_passwdqc.8
 
-WARNS?=	0
+WARNS?=	2
 CFLAGS+= -I${SRCDIR}
 
 DPADD=	${LIBCRYPT}