Re: [PATCH 2/4] pam_tally2: Fix unitialized use of fileinfo.st_size.

[Tomas Mraz <tmraz@xxxxxxxxxx>]

On St, 2014-08-13 at 14:26 +0200, Robin Hack wrote:
> ---
>  modules/pam_tally2/pam_tally2.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/modules/pam_tally2/pam_tally2.c b/modules/pam_tally2/pam_tally2.c
> index 09e8585..ba0781f 100644
> --- a/modules/pam_tally2/pam_tally2.c
> +++ b/modules/pam_tally2/pam_tally2.c
> @@ -368,6 +368,12 @@ get_tally(pam_handle_t *pamh, uid_t uid, const char *filename,
>  
>      if (*tfile != -1) {
>  	preopened = 1;
> +    lstat_ret = fstat(*tfile, &fileinfo);
> +    if (lstat_ret == -1) {
> +      /* If file is preopened, don't close file descriptor. */
> +      pam_syslog(pamh, LOG_ALERT, "Couldn't stat %s", filename);
> +      return PAM_AUTH_ERR;
> +    }
>  	goto skip_open;
>      }

Here again it is better to not use the fileinfo in the skip_open case at
all. It is better to just try pam_modutil_read and not to depend on
fileinfo.st_size.

Here is the patch:

diff --git a/modules/pam_tally2/pam_tally2.c b/modules/pam_tally2/pam_tally2.c
index 09e8585..f5eebb1 100644
--- a/modules/pam_tally2/pam_tally2.c
+++ b/modules/pam_tally2/pam_tally2.c
@@ -451,11 +451,8 @@ skip_open:
        alarm(oldalarm);
     }
 
-    if (fileinfo.st_size < (off_t)(uid+1)*(off_t)sizeof(*tally)) {
+    if (pam_modutil_read(*tfile, void_tally, sizeof(*tally)) != sizeof(*tally)) {
        memset(tally, 0, sizeof(*tally));
-    } else if (pam_modutil_read(*tfile, void_tally, sizeof(*tally)) != sizeof(*tally)) {
-       memset(tally, 0, sizeof(*tally));
-       /* Shouldn't happen */
     }
 
     tally->fail_line[sizeof(tally->fail_line)-1] = '\0';


-- 
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb
(You'll never know whether the road is wrong though.)


_______________________________________________
Pam-list mailing list
Pam-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/pam-list