#1849 improper use of negative value
Closed: Invalid None Opened 7 years ago by jhrozek.

Found by Coverity:

1280    if (lret != LDAP_SUCCESS) {
1281        DEBUG(3, ("ldap_search_ext failed: %s\n", sss_ldap_err2string(lret)));
At conditional (1): "lret == -1" taking the true branch.
CID 13155: Improper use of negative value (NEGATIVE_RETURNS)Variable "lret" tests negative.
1282        if (lret == LDAP_SERVER_DOWN) {
1283            ret = ETIMEDOUT;
1284            optret = sss_ldap_get_diagnostic_msg(state, state->sh->ldap,
1285                                                 &errmsg);
At conditional (2): "optret == 0" taking the false branch.
1286            if (optret == LDAP_SUCCESS) {
1287                DEBUG(3, ("Connection error: %s\n", errmsg));
1288                sss_log(SSS_LOG_ERR, "LDAP connection error: %s", errmsg);
1289            }
1290            else {
"lret" is passed to a parameter that cannot be negative. [hide details]
1291                sss_log(SSS_LOG_ERR, "LDAP connection error, %s",
/tmp/sssd/src/util/sss_ldap.c
33const char* sss_ldap_err2string(int err)
34{
At conditional (1): "(err & 0xffffffffffff0000) == 0x555d0000" taking the true branch.
At conditional (2): "err < 1432158224" taking the true branch.
35    if (IS_SSSD_ERROR(err)) {
Passing "err" to "sss_strerror", which cannot accept a negative. [show details]
36        return sss_strerror(err);
37    } else {
38        return ldap_err2string(err);
39    }
40}
41

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.10.0
rhbz: => 0
type: defect => task

/tmp/sssd/src/util/sss_ldap.c
33const char* sss_ldap_err2string(int err)
34{
At conditional (1): "(err & 0xffffffffffff0000) == 0x555d0000" taking the true branch.
^^^^^^^ ^^^^^^
If err will be negative number then this condition will NOT be true

EXPLANATION:

err = -9223372035422617596 // 0x80000000555d0004
SSSD_ERR_BASE(err) // -> expand_to: err & ~ERR_MASK
// -> expand_to: err & ~0x0000FFFF
// -> expand_to: err & 0xffffffffffff0000
// - result: 0x80000000555d0000
SSSD_ERR_BASE(err) == ERR_BASE // expand_to: 0x80000000555d0000 == 0x555d0000
// result: false

Macro IS_SSSD_ERROR(err) is defined as:
((SSSD_ERR_BASE(err) == ERR_BASE) && ((err) < ERR_LAST))
Therefore the first part of condition is false and thus result is false.

Coverity assumes, that sizeof(int) == 8, but even if sizeof(int) will be 4
IS_SSSD_ERROR will be false for negative values.

This coverity issue if false positive.

I investigated this issue and confirmed what Lukas wrote in previous comment. And even if the size of integer would be different, value passed in this particular occurence will always be -1 (thus taking correct branch)

Marking as invalid.

resolution: => invalid
status: new => closed

Metadata Update from @jhrozek:
- Issue set to the milestone: SSSD 1.10.0

3 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/2891

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