[Bug 1058] Updating protected password database in HP-UX

[bugzilla-daemon at mindrot.org]

http://bugzilla.mindrot.org/show_bug.cgi?id=1058





------- Additional Comments From dtucker at zip.com.au  2005-07-05 23:42 -------
(From update of attachment 936)
>+                pr=getprpwnam((char *)username);
>+                putpr=pr;

You're just copying a pointer to the struct not copying the struct itself.

>+                if(putpr != NULL) {
>+                       if(!putpr->uflg.fg_nlogins)
>+                               putpr->uflg.fg_nlogins=1;
>+                putpr->ufld.fd_nlogins++;

This will segfault if getprpwnam fails too.

>+                putprpwnam(username,putpr);

ditto.

You can take advantage of the fact that it's a function call and return early
rather than nesting your code (this tends to make it easier to read).  This
would look something like:

	struct pr_passwd *pr, putpr;

	if (!iscomsec())
		return;
	if ((pr = getprpwnam((char *)user)) == NULL)
		return;
	putpr = *pr;	/* copy and modify */
	putpr.uflg.fg_nlogins = 1;
	putpr.ufld.fd_nlogins++;
	putprpwnam((char *)user, &putpr);

Also this code is in auth.c which is one of the files we need to keep in sync,
it would be better elsewhere (auth-shadow.c is my first guess).

> #ifdef CUSTOM_FAILED_LOGIN
[...]
>+          if (authenticated == 0 && !authctxt->postponed && options.use_pam && strcmp(method, "none"))

Why "options.use_pam"?	If anything, I would have expected to skip this if PAM
is enabled.

>+               PRIVSEP(update_trusted_badlogins(authctxt->user));

This is not quite what I had in mind, but I now see that because we already use
record_failed_login() for btmp logging and can't use it directly.  Let me think
about it for a bit.

Also, are you ever clearing the counter on sucessful login?  Should you?




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.