Bug 1379560 - Uninitialized variable in gp_add_krb5_creds will cause seg-faults.
Summary: Uninitialized variable in gp_add_krb5_creds will cause seg-faults.
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: gssproxy
Version: 7.4
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: rc
: ---
Assignee: Robbie Harwood
QA Contact: Kaleem
URL:
Whiteboard:
Depends On:
Blocks: 1298243
TreeView+ depends on / blocked
 
Reported: 2016-09-27 07:02 UTC by Thomas Gardner
Modified: 2020-09-10 09:49 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-09-28 18:06:22 UTC
Target Upstream Version:


Attachments (Terms of Use)

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:


Note You need to log in before you can comment on or make changes to this bug.