From 713b65adaa756b5a66ccb0620d5f2bc50642f2f1 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Sat, 12 Jun 2010 16:07:52 -0700 Subject: [PATCH] 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 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- src/aklog/klog.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/aklog/klog.c b/src/aklog/klog.c index a509778..8c7606a 100644 --- 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 (;;) { -- 1.9.4