Avoid off-by-one error when saving the password in klog

When klog saved the password entered by the user to allow attempts
at multiple AFS principals without reprompting, it copied the whole
buffer according to the declared reply length into local storage.
This was done without regard to the local allocated storage size,
and was then nul-terminated without regard to the allocated storage
size.  Both klog and Heimdal use a size of BUFSIZ for the reply
buffer by default, which meant that klog on Heimdal was writing past
the end of the allocated structure when nul-terminating the password.

Store our allocated buffer size in the struct and only copy at most
one fewer than that many characters, and then nul-terminate
accordingly.

(The assumption that BUFSIZ is always long enough is still bogus,
but that's larger surgery.)

Change-Id: Ic8d4357aad2f8dfa0fffe9849d2546a88ecd246a
Reviewed-on: http://gerrit.openafs.org/2129
Tested-by: Russ Allbery <rra@stanford.edu>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/aklog/klog.c b/src/aklog/klog.c
index a5097783861815e64b5934934bb7ba0d0145e8d3..8c7606a2f9b7df5a4a0cb456bba4eb21115eb714 100644 (file)
--- a/src/aklog/klog.c
+++ b/src/aklog/klog.c
@@ -292,6 +292,7 @@ k5_to_k4_name(krb5_context k5context,
  */
 struct kp_arg {
     char **pp, *pstore;
+    size_t allocated;
 };
 krb5_error_code
 klog_prompter(krb5_context context,
@@ -307,6 +308,8 @@ klog_prompter(krb5_context context,
     krb5_prompt_type *types;
 #endif
     struct kp_arg *kparg = (struct kp_arg *) a;
+    size_t length;
+
     code = krb5_prompter_posix(context, a, name, banner, num_prompts, prompts);
     if (code) return code;
 #if !defined(USING_HEIMDAL) && defined(HAVE_KRB5_GET_PROMPT_TYPES)
@@ -333,8 +336,11 @@ klog_prompter(krb5_context context,
        switch(type) {
        case KRB5_PROMPT_TYPE_PASSWORD:
        case KRB5_PROMPT_TYPE_NEW_PASSWORD_AGAIN:
-           memcpy(kparg->pstore, prompts[i].reply->data, prompts[i].reply->length);
-           kparg->pstore[prompts[i].reply->length] = 0;
+           length = prompts[i].reply->length;
+           if (length > kparg->allocated - 1)
+               length = kparg->allocated - 1;
+           memcpy(kparg->pstore, prompts[i].reply->data, length);
+           kparg->pstore[length] = 0;
            *kparg->pp = kparg->pstore;
        }
     }
@@ -564,6 +570,7 @@ CommandProc(struct cmd_syndesc *as, void *arock)
 
     klog_arg->pp = &pass;
     klog_arg->pstore = passwd;
+    klog_arg->allocated = sizeof(passwd);
     /* XXX should allow k5 to prompt in most cases -- what about expired pw?*/
     krb5_get_init_creds_opt_init(gic_opts);
     for (;;) {