#1235 Two memory leaks in sss_sudo_get_values
Closed: Fixed None Opened 12 years ago by sgallagh.

The first one is fairly obvious (we need to call {{{free(values);}}} before returning ENOMEM.

130int sss_sudo_get_values(struct sss_sudo_rule *e,
131                        const char *attrname, char ***_values)
132{
133    struct sss_sudo_attr *attr = NULL;
134    char **values = NULL;
135    int i, j;
136
137    for (i = 0; i < e->num_attrs; i++) {
138        attr = e->attrs + i;
139        if (strcasecmp(attr->name, attrname) == 0) {
CID 12575: Resource leak (RESOURCE_LEAK)Calling allocation function "calloc".
Assigning: "values" = storage returned from "calloc(attr->num_values + 1U, 8UL)".
140            values = calloc(attr->num_values + 1, sizeof(char*));
At conditional (1): "values == NULL" taking the false branch.
141            if (values == NULL) {
142                return ENOMEM;
143            }
144
At conditional (2): "j < attr->num_values" taking the true branch.
145            for (j = 0; j < attr->num_values; j++) {
At conditional (3): "0" taking the false branch.
146                values[j] = strdup(attr->values[j]);
At conditional (4): "values[j] == NULL" taking the true branch.
147                if (values[j] == NULL) {
Variable "values" going out of scope leaks the storage it points to.
148                    return ENOMEM;
149                }
150            }

The second is more complicated. We are leaking memory each time we find an attr with the same name. This should be impossible, but we should be checking for it and failing the lookup if the {{{strcasecmp(attr->name, attrname)}}} matches more than once.

130int sss_sudo_get_values(struct sss_sudo_rule *e,
131                        const char *attrname, char ***_values)
132{
133    struct sss_sudo_attr *attr = NULL;
134    char **values = NULL;
135    int i, j;
136
At conditional (9): "i < e->num_attrs" taking the true branch.
137    for (i = 0; i < e->num_attrs; i++) {
138        attr = e->attrs + i;
At conditional (10): "strcasecmp(attr->name, attrname) == 0" taking the true branch.
139        if (strcasecmp(attr->name, attrname) == 0) {
Calling allocation function "calloc".
Assigning: "values" = storage returned from "calloc(attr->num_values + 1U, 8UL)".
Overwriting "values" in call "values = calloc(attr->num_values + 1U, 8UL)" leaks the storage that "values" points to.
140            values = calloc(attr->num_values + 1, sizeof(char*));
At conditional (1): "values == NULL" taking the false branch.
141            if (values == NULL) {
142                return ENOMEM;
143            }
144
At conditional (2): "j < attr->num_values" taking the true branch.
At conditional (5): "j < attr->num_values" taking the true branch.
At conditional (8): "j < attr->num_values" taking the false branch.
145            for (j = 0; j < attr->num_values; j++) {
At conditional (3): "0" taking the false branch.
At conditional (6): "0" taking the false branch.
146                values[j] = strdup(attr->values[j]);
At conditional (4): "values[j] == NULL" taking the false branch.
At conditional (7): "values[j] == NULL" taking the false branch.
147                if (values[j] == NULL) {
148                    return ENOMEM;
149                }
150            }
151
152            values[attr->num_values] = NULL;
153        }
154    }

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.8.1 (LTM)
owner: somebody => pbrezina
rhbz: => 0

Fields changed

patch: 0 => 1

Fixed by:
- ea155ef (master)
- 7f80db0 (sssd-1-8)

resolution: => fixed
status: new => closed

Metadata Update from @sgallagh:
- Issue assigned to pbrezina
- Issue set to the milestone: SSSD 1.8.1 (LTM)

7 years ago

SSSD is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in SSSD's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/SSSD/sssd/issues/2277

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Login to comment on this ticket.

Metadata