Attachment 76619 Details for Bug 155003

[patch] novell-lum.diff

novell-lum.diff (text/plain), 10.25 KB, created by Thomas Biege on 2006-04-05 09:25:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Thomas Biege

Created: 2006-04-05 09:25:37 UTC

Size: 10.25 KB

patch

obsolete

>@@ -243,40 +262,71 @@ int flush_userHash_to_file(DBM *dbm_ptr,
> 	/* format is name:x:uid:gid:gecos:dir:shell:userfdn:passwdHash:number of gids:
> 				 gid1:gid2:gid3...... 		
> 	*/
>	// XXX tom: be paranoid, better check if len > 0
>-	sprintf(str,"%s:%s:%x:%x:",userHashEntry->pwd->pw_name,
>+	snprintf(str,len-1,"%s:%s:%x:%x:",userHashEntry->pwd->pw_name,
>
>@@ -366,12 +418,13 @@ int flush_groupHash_to_file(DBM *dbm_ptr
> 		syslog(LOG_DEBUG,"Unable to allocate memory for group string");
> 		return MALLOC_ERROR;
> 	}
>+	memset(str,0,len);
> 
> 	/* format is name:x:gid:member_count:member1:member2:....	*/
> 	sptr = str;
>	// XXX tom: be paranoid and check for len > 0 :)
>-	sprintf(str,"%s:%s:%x:%x:",groupHashEntry->grp->gr_name,
>+	snprintf(str,len-1, "%s:%s:%x:%x:",groupHashEntry->grp->gr_name,
>
>diff -upEbr novell-lum.bak/novell-lum-2.2.0/namcd/nss_ldap.c novell-lum/novell-lum-2.2.0/namcd/nss_ldap.c
>--- novell-lum.bak/novell-lum-2.2.0/namcd/nss_ldap.c	2006-03-27 14:48:57.000000000 +0200
>+++ novell-lum/novell-lum-2.2.0/namcd/nss_ldap.c	2006-03-29 06:37:06.000000000 +0200
>@@ -1223,7 +1223,7 @@ NSS_STATUS getGrpbyName_fromNDS(nds_nss_
>     return error;
>   }
> 
>-  sprintf(filter, "%s%s%s%s%s","(&(objectclass=", UNIXGROUPOBJECT, ")(cn=", name, "))");// XXX tom: snprintf()?
>+  sprintf(filter, "%s%s%s%s%s","(&(objectclass=", UNIXGROUPOBJECT, ")(cn=", name, "))");
> 
>   if ((status = searchAndFillGrpStruct_fromNDS(nss_priv->ldapprivate.ld, 
>                                                LDAP_SCOPE_BASE, 
>@@ -2955,7 +2955,7 @@ NSS_STATUS populate_userHash_fromNDS(nds
> 
>     if ((dn = ldap_get_dn(nss_priv->ldapprivate.ld, e)) != NULL)
>     {
>-      fdn = (char*)calloc((strlen(dn)+1), sizeof(char)); // XXX tom: check for NULL
>+      fdn = (char*)calloc((strlen(dn)+1), sizeof(char));
>       strcpy(fdn, dn);
>       ldap_memfree(dn);
>     }
>diff -upEbr novell-lum.bak/novell-lum-2.2.0/namcd/requests.c novell-lum/novell-lum-2.2.0/namcd/requests.c
>--- novell-lum.bak/novell-lum-2.2.0/namcd/requests.c	2006-03-23 20:59:01.000000000 +0100
>+++ novell-lum/novell-lum-2.2.0/namcd/requests.c	2006-03-29 00:43:09.000000000 +0200
>@@ -206,14 +206,14 @@ int readFromSock(int sockfd, int flag, i
> 	if (flag==LENANDDATA)
> 	{
> 		int nb = *len,nbr;
>-		*name = (char*)malloc(nb); // XXX tom: why should we trust this value?
>+		*name = (char*)malloc(nb);
> 		if (NULL == *name)
> 		{
> 			syslog(LOG_ERR, "readFromSock: Unable to allocate memory.");
> 			return MALLOC_ERROR;
> 		}
> 		/* read the name from socket */
>-		if ((nbr = _nds_nss_readFromSock(sockfd,*name, *len, NORESPONSE_TIMEOUT)) != nb) // XXX tom: can this overflow *name?
>+		if ((nbr = _nds_nss_readFromSock(sockfd,*name, *len, NORESPONSE_TIMEOUT)) != nb)
> 		{
> 			syslog(LOG_ERR,"readFromSock: Error reading from socket (len and data), errno=%d.",errno);
> 			//write_errToSock(sockfd,errno);
>@@ -245,7 +245,7 @@ int readFromSockExt(int sockfd, int *len
> #endif
> 
> 	nb = *len;
>-	*name = (char*)malloc(nb); // XXX tom: untrusted value
>+	*name = (char*)malloc(nb);
> 
> 	if (*name == NULL)
> 	{
>@@ -255,7 +255,7 @@ int readFromSockExt(int sockfd, int *len
> 	}
> 
> 	/* read the name from the socket */
>-	if ((nbr = _nds_nss_readFromSock(sockfd, *name, *len, NORESPONSE_TIMEOUT)) != nb) // XXX tom: possible overflow
>+	if ((nbr = _nds_nss_readFromSock(sockfd, *name, *len, NORESPONSE_TIMEOUT)) != nb)
> 	{
> 		syslog(LOG_ERR,"readFromSockExt: Error reading name from the socket, errno=%d.",errno);
> 		*name = NULL;
>@@ -278,10 +278,10 @@ int readFromSockExt(int sockfd, int *len
> 	syslog(LOG_DEBUG,"readFromSockExt: Password length = %d.", *lenForPass);
> #endif
> 
>-	nb = *lenForPass; // XXX tom: untrusted value
>+	nb = *lenForPass;
> 	if (nb != 0 )
> 	{
>-		*pass = (char*)malloc(nb); // XXX tom: can result in DoS, let nb be -1
>+		*pass = (char*)malloc(nb);
> 
> 		if (*pass == NULL)
> 		{
>
>@@ -354,7 +284,7 @@ static int copy_tree(const char *src_roo
> 			err++;
> 			break;
> 		}
>-		if ((ofd = open (dst_name, O_WRONLY|O_CREAT, 0)) < 0 || // XXX tom: this will follow sym links. a problem?
>+		if ((ofd = open (dst_name, O_WRONLY|O_CREAT, 0)) < 0 ||
> 			chown (dst_name, uid == (uid_t) -1 ? sb.st_uid:uid,
> 					gid == (gid_t) -1 ? sb.st_gid:gid) ||
> 				chmod (dst_name, sb.st_mode & 07777)) {
>diff -upEbr novell-lum.bak/novell-lum-2.2.0/pam/ntsolsync.c novell-lum/novell-lum-2.2.0/pam/ntsolsync.c
>--- novell-lum.bak/novell-lum-2.2.0/pam/ntsolsync.c	2006-03-16 12:18:17.000000000 +0100
>+++ novell-lum/novell-lum-2.2.0/pam/ntsolsync.c	2006-03-29 00:43:09.000000000 +0200
>@@ -68,7 +68,7 @@ int SetLMandNTpassword(LDAP *handle,char
>   //attrValues=(DDCVALUE *)malloc(SIZE);
>   //currentattributeValuePointer=attrValues;
>   //memset(attrValues,0,SIZE);
>-  strcpy(password,newpassword); // XXX tom: can we be sure that it fits in the buffer?
>+  strcpy(password,newpassword);
> 
>   //Get the Lan Manager password hash to store in user attribute
>     HashLMpassword(password, passwordB,passwordD);
>@@ -445,7 +445,7 @@ int    HashNTpassword(pnstr8 newPassword
>       &messageDigest);
> 
> 
>-      /* Encrypt the NT OWF password using a random value as the key. // XXX tom: if this really need to be random rand() will not work. same situation on other places
>+      /* Encrypt the NT OWF password using a random value as the key.
>       */
>    des_ecb_encrypt_with_index(
>       (PDES_CLEAR_BLOCK)&messageDigest,
>diff -upEbr novell-lum.bak/novell-lum-2.2.0/pam/pam_ap.c novell-lum/novell-lum-2.2.0/pam/pam_ap.c
>--- novell-lum.bak/novell-lum-2.2.0/pam/pam_ap.c	2006-03-17 14:32:11.000000000 +0100
>+++ novell-lum/novell-lum-2.2.0/pam/pam_ap.c	2006-03-29 00:43:09.000000000 +0200
>@@ -65,7 +65,7 @@ static void _pam_log (int error, const c
> 	openlog (identification, LOG_PID, LOG_AUTH);
> 	/*  openlog (identification, LOG_PID, LOG_LOCAL0); */
> 	setlogmask (LOG_UPTO (LOG_DEBUG));
>-	syslog (error, buffer); // XXX tom: possible fmt str bug when called with user supplied data
>+	syslog (error, buffer);
> 	closelog ();
> }
> 
>diff -upEbr novell-lum.bak/novell-lum-2.2.0/pam/pamfunc.c novell-lum/novell-lum-2.2.0/pam/pamfunc.c
>--- novell-lum.bak/novell-lum-2.2.0/pam/pamfunc.c	2006-03-20 16:13:37.000000000 +0100
>+++ novell-lum/novell-lum-2.2.0/pam/pamfunc.c	2006-03-29 02:47:16.000000000 +0200
>@@ -531,7 +531,7 @@ extern int pam_sm_authenticate(pam_handl
> 
> 			if (isPAMServiceOnExcludeList(pamh, userFDN, service, userinfo.pw_gid) == 0)
> 			{
>-				sprintf(infomsg,PAM_EXCLUDE_LIST, service, username); // XXX tom: buffer overflow?
>+				sprintf(infomsg,PAM_EXCLUDE_LIST, service, username);
> 				_nds_pam_display_converse(pamh, PAM_TEXT_INFO, infomsg,NORESPFLAG,dummychar1);
> #ifdef DEBUG
> 				syslog(LOG_ERR,"PAM_NAM: pam_sm_authenticate(): PAM service for [%s] is excluded for user [%s].\n", service, username);
>@@ -558,7 +558,7 @@ extern int pam_sm_authenticate(pam_handl
> 			return PAM_SUCCESS;
> 
> 		case ERR_PASSWD_EXPIRED:
>-			return PAM_SUCCESS; // XXX tom: is that the right behavior?
>+			return PAM_SUCCESS;
> 
> 		case ERR_LOGIN_LOCKOUT:
> 		case ERR_MAXIMUM_LOGINS_EXCEEDED:
>@@ -1483,7 +1315,7 @@ extern int pam_sm_chauthtok(pam_handle_t
> 
> 				if (_nds_pam_read_options(argc,(const char **)argv,storeoldpass,OPTIONS) == PAM_SUCCESS)
> 				{
>-					strcpy(oldpass,storeoldpass); // XXX tom: does it fit?
>+					strcpy(oldpass,storeoldpass);
> 				}
> /* do a NULL bind here if the connection state has not been stored 
> *by the auth module*/
>diff -upEbr novell-lum.bak/novell-lum-2.2.0/pam/pam_misc.c novell-lum/novell-lum-2.2.0/pam/pam_misc.c
>--- novell-lum.bak/novell-lum-2.2.0/pam/pam_misc.c	2006-03-21 15:27:18.000000000 +0100
>+++ novell-lum/novell-lum-2.2.0/pam/pam_misc.c	2006-03-29 23:22:04.000000000 +0200
>@@ -111,7 +111,7 @@ int _nds_pam_display_converse(pam_handle
> 	{
> 		if (resp && resp->resp)
> 		{
>-			strcpy(msgresp, resp->resp); // XXX tom: does it fit?
>+			strcpy(msgresp, resp->resp);
> 			free(resp->resp);
> 			free(resp);
> 			return PAM_SUCCESS;
>@@ -141,7 +141,7 @@ int _nds_pam_pass_converse(pam_handle_t 
> 	{
> 		if (resp && resp->resp)
> 		{
>-			strcpy(msgresp1, resp->resp); // XXX tom: does it fit?
>+			strcpy(msgresp1, resp->resp);
> 			pmsg[0] = &msg[0];
> 			msg[0].msg_style = msgstyle;
> 			msg[0].msg = msgdisp2;
>@@ -149,7 +149,7 @@ int _nds_pam_pass_converse(pam_handle_t 
> 			{
> 				if (resp1 && resp1->resp)
> 				{
>-					strcpy(msgresp2, resp1->resp); // XXX tom: does it fit?
>+					strcpy(msgresp2, resp1->resp);
> 					free(resp);
> 					free(resp1);
> 					return PAM_SUCCESS;
>@@ -239,7 +239,7 @@ void _nds_cleanup_chars(pam_handle_t *pa
> 
> void init_sec_salt(char *sec_salt)
> {
>-	sec_salt[0] = 'k'; // XXX tom: a constant salt is a non-existing salt
>+	sec_salt[0] = 'k';
> 	sec_salt[1] = 'y';
> 	sec_salt[2] = '\0';
> 	return;
>@@ -353,7 +353,7 @@ int search(LDAP *handle,char *userFDN, c
> 				for ( i = 0; values[ i ] != NULL; i++ )
> 				{
> 					if (strcmp(attribute,attr[0])==0)
>-						strcpy(val,values[i]); // XXX tom: does it fit?
>+						strcpy(val,values[i]);
> 				}
> 				ldap_value_free( values );
> 			}
>@@ -450,7 +450,7 @@ int _nds_GetUnixKeyForUser(pam_handle_t 
> 				for ( i = 0; values[ i ] != NULL; i++ )
> 				{
> 					if (strcmp(attribute,attrs[0])==0)
>-						strcpy(usrkey,values[i]); // XXX tom: does it fit?
>+						strcpy(usrkey,values[i]);
> 				}
> 				ldap_value_free( values );
> 			}
>@@ -866,7 +866,7 @@ int LIBCALL cert_callback( void *pHandle
> 		goto err;
> 	}
> 
>-	cert.data = (void *)malloc(cert.length); // XXX tom: can we trust length? looking at NLDAPsdk revealed that we can not :(
>+	cert.data = (void *)malloc(cert.length);
> 	if (NULL == cert.data)
> 	{
> 		syslog(LOG_ERR,"cert_callback():Could not allocate buffer for obtaining cert");
>
>
>@@ -1420,7 +1420,7 @@ NSS_STATUS search_database(char *cn,char
> 	int found=0;
> 	char temp[MAX_ARRYLEN], *value;
> 	FILE *fp;
>-	fp=fopen("/CNtoFDN","r"); // XXX tom: what is that?
>+	fp=fopen("/CNtoFDN","r");
> 	fseek(fp,0L,SEEK_SET);
> 	while (!feof(fp))
> 	{
>@@ -1450,7 +1450,7 @@ void itoa(int n, char s[], int radix)
> {
> 	int i,sign,c,j;
> 
>-	radix = radix; // XXX tom: ??? :)
>+	radix = radix;
> 
> 	if ((sign =n)<0)
> 		n=-n;
>
>@@ -2041,7 +2041,7 @@ int setPasswordForName(const char *userN
> 
> int CryptPasswordForCache(const char *password, char *cryptPass)
> {
>-  char salt[] = "$1$LLBSALT$"; // XXX tom: constant salt!
>+  char salt[] = "$1$LLBSALT$";
>   char *temp, buffer[128];
> 
>   temp = buffer;
>@@ -2799,7 +2799,7 @@ int namGetUserFDNfromUIDCacheOnly(int ui
> 		return errorval;
> 	}
> 
>-	strcpy(userFDN, pstr); // XXX tom: does it fit?
>+	strcpy(userFDN, pstr);
> 
> 	free(buffer);
> 	return NAM_SUCCESS;

Actions: View | Diff

Attachments on bug 155003: 74859 | 74920 | 75134 | 75596 | 76619