#1849 improper use of negative value
Closed: Invalid None Opened 6 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

2 years ago

Login to comment on this ticket.

Metadata