#1333 Potential NULL dereference in proxy provider
Closed: Fixed None Opened 7 years ago by jhrozek.

The very first condition is reversed, it must say if (lowercase || alias).

At conditional (3): "!lowercase" taking the false branch.
At conditional (4): "alias" taking the false branch.
 218    if (!lowercase || alias) {
 219        attrs = sysdb_new_attrs(NULL);
 220        if (!attrs) {
 221            DEBUG(SSSDBG_CRIT_FAILURE, ("Allocation error ?!\n"));
 222            return ENOMEM;
 223        }
 224    }
 225
At conditional (5): "lowercase" taking the true branch.
 226    if (lowercase) {
 227        lower = sss_tc_utf8_str_tolower(attrs, pwd->pw_name);
At conditional (6): "!lower" taking the false branch.
 228        if (!lower) {
 229            DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot convert name to lowercase\n"));
 230            talloc_zfree(attrs);
 231            return ENOMEM;
 232        }
 233
Passing null variable "attrs" to function "sysdb_attrs_add_string", which dereferences it. [show details]
 234        ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, lower);

Fields changed

keywords: => Coverity easyfix

Lowering the priority, upon further inspection I realized that there can't be a case where "alias" is NULL in the current code, so the attrs are always created (which also explains why neither me nor QE hit the potential crash).

We should still get it fixed, though. It's a logic bug.

priority: major => minor

Fields changed

owner: somebody => arielb

Fields changed

status: new => assigned

Fixed by:
- c5eb0dc (master)
- 0078eb3 (sssd-1-8)

component: SSSD => Proxy Provider
milestone: NEEDS_TRIAGE => SSSD 1.8.4 (LTM)
patch: 0 => 1
resolution: => fixed
status: assigned => closed

Fields changed

rhbz: => 0

Metadata Update from @jhrozek:
- Issue assigned to arielb
- Issue set to the milestone: SSSD 1.8.4 (LTM)

2 years ago

Login to comment on this ticket.

Metadata