Bug 1379560

Summary: Uninitialized variable in gp_add_krb5_creds will cause seg-faults.
Product: Red Hat Enterprise Linux 7 Reporter: Thomas Gardner <thgardne>
Component: gssproxyAssignee: Robbie Harwood <rharwood>
Status: CLOSED NOTABUG QA Contact: Kaleem <ksiddiqu>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 7.4CC: fs-qe, thgardne, yoyang
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-09-28 18:06:22 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 1298243    

Description Thomas Gardner 2016-09-27 07:02:42 UTC
Description of problem:

In /src/gp_creds.c, in gp_add_krb5_creds, the local varable cred_store
is never initialized.  A pointer to it is first passed into
gp_get_cred_environment.  In that function:

    329     /* allocate 1 more than in source, just in case we need to add
    330      * an internal client_keytab element */
    331     cs->elements = calloc(svc->krb5.cred_count + 1,
    332                           sizeof(gss_key_value_element_desc));
    333     if (!cs->elements) {
    334         ret = ENOMEM;
    335         goto done;
    336     }

and the "done" label goes like:

    410 done:
    411     if (ret) {
    412         free_cred_store_elements(cs);
    413     }
    414     return ret;
    415 }

and free_cred_store_elements looks like:

    206 static void free_cred_store_elements(gss_key_value_set_desc *cs)
    207 {
    208     int i;
    209 
    210     for (i = 0; i < cs->count; i++) {
    211         safefree(cs->elements[i].key);
    212         safefree(cs->elements[i].value);
    213     }
    214     safefree(cs->elements);
    215 }

Following this code path, cs->count is never initialized, and whatever
random crud that happens to be on the stack will be taken as a counter,
and whatever random crud happens to on the stack be where
cs->elements[i] is located will be interpreted as a pointer to a key.
Likewise with cs->elements[i].value.

It would be wise to simply initialize the variable cred_store in
gp_get_cred_environment to {0, NULL}.

Version-Release number of selected component (if applicable):


How reproducible:

100%.

Steps to Reproduce:
1.  Look at the code.
2.  See the bug.
3.  Slap self in forehead.

Actual results:

Seg-fault from dereferencing a random number as a pointer.

Expected results:

No seg-fault.

Additional info: