Static analysis reports garbage value in _lmatch

[wolframroesler]

I'm validating mDNS with clang-tidy and fixed a lot of findings already, but there's one that requires more intimate knowledge of the _lmatch function than I have. The reported problem is:

warning: The left operand of '==' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]

Here's the full checker output:

clang-tidy-9 -header-filter=.* -p=/home/wolfram/fw/sourceSDK/build -quiet -config={"Checks":"*,-android-*,-llvm-include-order,-llvm-header-guard,-readability-else-after-return,-readability-magic-numbers,-readability-inconsistent-declaration-parameter-name,-readability-isolate-declaration,-readability-braces-around-statements,-readability-named-parameter,-cppcoreguidelines-avoid-magic-numbers,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-hicpp-signed-bitwise,-hicpp-braces-around-statements,-hicpp-multiway-paths-covered,-hicpp-no-assembler,-google-readability-braces-around-statements,-google-readability-todo,-bugprone-suspicious-string-compare,-cert-msc30-c,-cert-msc50-cpp","CheckOptions":[{"key":"misc-throw-by-value-catch-by-reference.CheckThrowTemporaries","value":"1"}]} /home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:179:10: warning: The left operand of '==' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
        if (*l1 == 0 && *l2 == 0)
                ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:465:12: note: Calling '_host'
        short2net(_host(m, &(m->_buf), name) + 6, &mybuf);
                  ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:192:6: note: Assuming 'name' is not equal to null
        if (name == 0)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:192:2: note: Taking false branch
        if (name == 0)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:196:2: note: Loop condition is true.  Entering loop body
        while (name[y]) {
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:197:7: note: Assuming the condition is false
                if (name[y] == '.') {
                    ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:197:3: note: Taking false branch
                if (name[y] == '.') {
                ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:206:3: note: Taking false branch
                if (x++ == 255)
                ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:196:2: note: Loop condition is false. Execution continues on line 212
        while (name[y]) {
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:213:6: note: 'x' is not equal to 1
        if (x == 1)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:213:2: note: Taking false branch
        if (x == 1)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:219:2: note: Loop condition is true.  Entering loop body
        for (x = 0; label[x]; x += label[x] + 1) {
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:220:15: note: 'y' is < MAX_NUM_LABELS
                for (y = 0; y < MAX_NUM_LABELS && m->_labels[y]; y++) {
                            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:220:15: note: Left side of '&&' is true
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:220:3: note: Loop condition is true.  Entering loop body
                for (y = 0; y < MAX_NUM_LABELS && m->_labels[y]; y++) {
                ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:221:8: note: Calling '_lmatch'
                        if (_lmatch(m, label + x, m->_labels[y])) {
                            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:157:2: note: Taking false branch
        if (*l1 & 0xc0)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:159:6: note: Assuming the condition is false
        if (*l2 & 0xc0)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:159:2: note: Taking false branch
        if (*l2 & 0xc0)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:163:6: note: 'l1' is not equal to 'l2'
        if (l1 == l2)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:163:2: note: Taking false branch
        if (l1 == l2)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:167:6: note: Assuming the condition is false
        if (*l1 != *l2)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:167:2: note: Taking false branch
        if (*l1 != *l2)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:169:2: note: Loop condition is true.  Entering loop body
        for (len = 1; len <= *l1; len++) {
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:170:7: note: Assuming the condition is false
                if (l1[len] != l2[len])
                    ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:170:3: note: Taking false branch
                if (l1[len] != l2[len])
                ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:169:2: note: Loop condition is false. Execution continues on line 175
        for (len = 1; len <= *l1; len++) {
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:179:6: note: Left side of '&&' is true
        if (*l1 == 0 && *l2 == 0)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:179:18: note: Assuming the condition is false
        if (*l1 == 0 && *l2 == 0)
                        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:179:2: note: Taking false branch
        if (*l1 == 0 && *l2 == 0)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:183:9: note: Calling '_lmatch'
        return _lmatch(m, l1, l2);
               ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:157:2: note: Taking false branch
        if (*l1 & 0xc0)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:159:6: note: Assuming the condition is true
        if (*l2 & 0xc0)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:159:2: note: Taking true branch
        if (*l2 & 0xc0)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:160:10: note: Calling '_lmatch'
                return _lmatch(m, l1, (char *)m->_buf + _ldecomp(l2));
                       ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:157:2: note: Taking false branch
        if (*l1 & 0xc0)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:159:6: note: Assuming the condition is false
        if (*l2 & 0xc0)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:159:2: note: Taking false branch
        if (*l2 & 0xc0)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:163:6: note: 'l1' is not equal to 'l2'
        if (l1 == l2)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:163:2: note: Taking false branch
        if (l1 == l2)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:167:6: note: Assuming the condition is false
        if (*l1 != *l2)
            ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:167:2: note: Taking false branch
        if (*l1 != *l2)
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:169:2: note: Loop condition is false. Execution continues on line 175
        for (len = 1; len <= *l1; len++) {
        ^
/home/wolfram/fw/sourceSDK/core/src/apps/mdns/libmdnsd/1035.c:179:10: note: The left operand of '==' is a garbage value
        if (*l1 == 0 && *l2 == 0)
                ^

Please note that line numbers refer to our copy of 10.35.c which is not necessarily identical to the one on master.

[troglobit]

Hmm, I'm not the original author of that code, but it sure does look suspicious, and quite dangerous. Personally I'd go back to the spec and rewrite that code altogether ...

I just checked with Coverity Scan, it doesn't trigger on that part of the code, but the function preceding it; _label(). I've sent you guys an invite to view Coverity findings, if you're interested.

[wolframroesler]

Thanks, looking forward to your implementation. I must admit that I have only a vague idea of what's going on in these functions.

[troglobit]

Sorry, I meant to say that's what I would do, but I don't have the time right now to do it. Sorry for my being unclear.