1256169 - NSS scan-build issues in derive.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Eric Rescorla (:ekr)]

These seem sort of concerning. Some of them turn failure into success.

/Users/ekr/l/checker-278/bin/../libexec/ccc-analyzer -o Darwin15.3.0_ccc-analyzer_64_DBG.OBJ/derive.o -c -g -fPIC  -fno-common -pipe -DDARWIN -DHAVE_STRERROR -DHAVE_BSD_FLOCK  -Wall -Qunused-arguments -Wno-parentheses-equality -Werror -DXP_UNIX -DDEBUG -UNDEBUG -DDEBUG_ekr -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DNSS_ENABLE_TLS_1_3 -DNSS_SSL_ENABLE_ZLIB -I../../../dist/Darwin15.3.0_ccc-analyzer_64_DBG.OBJ/include -I../../../dist/public/nss -I../../../dist/private/nss  derive.c
derive.c:378:9: warning: Value stored to 'i' is never read
        i += effKeySize;
        ^    ~~~~~~~~~~
derive.c:636:5: warning: Value stored to 'rv' is never read
    rv = SECFailure;
    ^    ~~~~~~~~~~
derive.c:690:17: warning: Value stored to 'rv' is never read
                rv = SECSuccess;
                ^    ~~~~~~~~~~
derive.c:817:21: warning: Value stored to 'rv' is never read
                    rv = SECFailure;
                    ^    ~~~~~~~~~~
derive.c:836:17: warning: Value stored to 'rv' is never read
                rv = SECFailure;
                ^    ~~~~~~~~~~
5 warnings generated.

Comment 1

[Daniel Veditz [:dveditz]]

Potentially sec-high without knowing the code. If someone investigates and finds these are less damaging we can lower the rating.

Comment 2

[Franziskus Kiefer [:franziskus]]

This code is going the be removed soonish in bug 1252849. So I don't think it is worth investing time here.

Comment 3

[Franziskus Kiefer [:franziskus]]

Being for export grade ciphers only this also never used in Firefox. I'd consider this sec-low.

Comment 4

[Andrew McCreight [:mccr8]]

Marking low per comment 3.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Confirming sec-low, potentially even lower. Some of these are invalid, and some might be possible to exploit when we're OOM but even then we probably wouldn't correctly establish the connection.

More importantly, all of these failures are in the pkcs11Bypass code, which is ifdef'ed out in Firefox and is slated for removal in the near future.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1256169-Fix-a-few-scan-build-issues-in-derive.c.patch

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/8d884c514bf4