Closed Bug 133702 Opened 22 years ago Closed 22 years ago

Bugs discovered by using the anti-c lint engine.

Categories

(NSS :: Libraries, defect, P2)

Product:
NSS
Component:
Libraries
Platform:
x86
All
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: cyent, Assigned: bishakhabanerjee)

Details

Attachments

(1 file, 1 obsolete file)

Addresses comment 5 concerns
6.05 KB, patch
Details | Diff | Splinter Review 
This is a list of bugs found by antic (http://artho.com/jlint/)

The vast majority of false positives have been removed leaving only
definite bugs and highly suspicious cases.

======================================================================
Case 5 will be overwritten. 
mozilla/security/nss/cmd/crmftest/testcrmf.c:1270:5: Possible miss of BREAK
before CASE/DEFAULT

    switch (dbVersion) {
    case 7:
       dbver = "7";
       break;
    case 6:
       dbver = "6";
       break;
    case 5:
        dbver = "5";
    case 4:
    default:
        dbver = "";
	break;
======================================================================
In case crmfSubsequentMessage, rv will get overridden with SECFailure

mozilla/security/nss/lib/crmf/crmfpop.c:597:5: Possible miss of BREAK before
CASE/DEFAULT

    case crmfSubsequentMessage:
        rv = crmf_add_privkey_subseqmessage(inCertReqMsg, subseqMess, 
					    crmfKeyAgreement);
    case crmfDHMAC:
        /* This case should be added in the future. */
    default:
        rv = SECFailure;
    }
======================================================================
In case SEC_OID_X942_DIFFIE_HELMAN_KEY: keyType gets overridden by "nullKey"

mozilla/security/nss/lib/cryptohi/seckey.c:883:7: Possible miss of BREAK before
CASE/DEFAULT


      case SEC_OID_X942_DIFFIE_HELMAN_KEY:
	keyType = dhKey;
      default:
======================================================================
Somebody doesn't like Diffie Helman....
mozilla/security/nss/lib/pk11wrap/pk11cert.c:1614:5: Possible miss of BREAK
before CASE/DEFAULT

    case dhKey:
        newItem = SECITEM_DupItem(&pubk->u.dh.publicValue);
    case fortezzaKey:
    default:
	newItem = NULL; /* Fortezza Fix later... */
======================================================================
This switch is too hairy for me to tell if this really is a bug, but
it looks suspicious...
mozilla/security/nss/lib/pk11wrap/pk11pbe.c:536:5: Possible miss of BREAK before
CASE/DEFAULT
    case pbeBitGenCipherIV:
	if (bitsNeeded > 64) {
	    break;
	}
	if (hashAlgorithm != SEC_OID_SHA1) {
	    break;
	}
	mechanism = CKM_PBE_SHA1_DES3_EDE_CBC;
    case pbeBitGenCipherKey:

======================================================================
This code is never reached.....

mozilla/security/nss/lib/pkix/src/AttributeTypeAndValue/PCreateFromUTF8.c:113:3:
Unreachable statement

  if( (nssArenaMark *)NULL != mark ) {
    if( PR_SUCCESS != nssArena_Unmark(arena, mark) ) {
      goto loser;
    }
  }
======================================================================
And again...
mozilla/security/nss/lib/pkix/src/Name/PCreateFromUTF8.c:115:3: Unreachable
statement
And again....
mozilla/security/nss/lib/pkix/src/RelativeDistinguishedName/PCreateFromUTF8.c:115:3:
Unreachable statement

  if( (nssArenaMark *)NULL != mark ) {
    if( PR_SUCCESS != nssArena_Unmark(arena, mark) ) {
      goto loser;
    }
  }
======================================================================
Classic missing "break"s....
mozilla/security/nss/lib/smime/smimeutil.c:249:9: Possible miss of BREAK before
CASE/DEFAULT
    switch (algtag) {
    case SEC_OID_RC2_CBC:
	keylen_bits = PK11_GetKeyStrength(key, algid);
	switch (keylen_bits) {
	case 40:
	    c = SMIME_RC2_CBC_40;
	case 64:
	    c = SMIME_RC2_CBC_64;
	case 128:
	    c = SMIME_RC2_CBC_128;
	default:
	    rv = SECFailure;
	    break;
	}
	break;
    case SEC_OID_DES_CBC:
	c = SMIME_DES_CBC_56;
    case SEC_OID_DES_EDE3_CBC:
	c = SMIME_DES_EDE3_168;
    case SEC_OID_FORTEZZA_SKIPJACK:
	c = SMIME_FORTEZZA;
    default:
	rv = SECFailure;
    }
======================================================================
Possible missing break? Looks suspicious....
mozilla/security/nss/lib/softoken/pkcs11.c:984:5: Possible miss of BREAK before
CASE/DEFAULT

    case CKK_DSA:
	if ( !pk11_hasAttribute(object, CKA_SUBPRIME)) {
	    return CKR_TEMPLATE_INCOMPLETE;
	}
    case CKK_DH:
======================================================================
And again.... I'm not sure about this one...
mozilla/security/nss/lib/softoken/pkcs11.c:1117:5: Possible miss of BREAK before
CASE/DEFAULT

    case CKK_DSA:
	if ( !pk11_hasAttribute(object, CKA_SUBPRIME)) {
	    return CKR_TEMPLATE_INCOMPLETE;
	}
	if ( !pk11_hasAttribute(object, CKA_NETSCAPE_DB)) {
	    return CKR_TEMPLATE_INCOMPLETE;
	}
    case CKK_DH:
======================================================================
This one looks bad...
mozilla/security/nss/lib/softoken/pkcs11c.c:2317:5: Possible miss of BREAK
before CASE/DEFAULT
    case CKM_TLS_PRF_GENERAL:
	crv = pk11_TLSPRFInit(context, key, key_type);

    default:
	crv = CKR_MECHANISM_INVALID;
	break;

======================================================================
Possible bug, perhaps not....
mozilla/security/nss/lib/ssl/sslsock.c:743:7: Possible miss of BREAK before
CASE/DEFAULT

      case SSL_ENABLE_FDX:
      	ssl_defaults.fdx = on;

======================================================================
Also occurs in nsprpub
If USE_REENTRANT_LIBC is defined, fprint of error will occur whether
error or not. Solution: Put braces round it.


mozilla/security/coreconf/nsinstall/pathsub.c:76:9: May be wrong assumption
about IF body

    if (error)

#ifdef USE_REENTRANT_LIBC
    R_STRERROR_R(errno);
	fprintf(stderr, ": %s", r_strerror_r);
#else
	fprintf(stderr, ": %s", strerror(errno));
#endif
======================================================================
Assigned the bug to Bishakha.  These warnings need to be reviewed
carefully.  They are not necessarily bugs.
Assignee: wtc → bishakhabanerjee
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Target Milestone: --- → 3.4.1
Changed the QA contact to Bishakha.
QA Contact: sonja.mirtitsch → bishakhabanerjee
Set target milestone to NSS 3.5.
Target Milestone: 3.4.1 → 3.5
Attached patch Applies some of the suggestions (obsolete) — — Splinter Review 
Just mindlessly turning some of the suggestions into patches. I left anything
that was "unsure" out of the patch.  Not that everything in the patch is
necessarily correct, but it gives something to hold discussions over =)
Review comments on the foregoing patch:

Each of these changes appears correct, as far as they go.  However, one of 
them, the change to smimeutil.c, only fixes 3 of the 6 shown switch cases 
that are missing breaks.

Comments on the items not addressed by this patch:

The missing break in pk11pbe, just prior to case pbeBitGenCipherKey
appears to be a bug.

The missing break in sslsock.c is a bug.  The case is probably unused.

Both of the two places shown in pkcs11.c where case CKK_DSA falls 
through into case CKK_DH are correct and intentional.  Some comment like 
/* fall through */ should be added to make this apparent

Re: the unreachable code shown in two copies of PCreateFromUTF8.c,
This code looks unfinished (incomplete).  Is it even being built?
I'd say we shouldn't attempt to delint this code until it's finished.

        Attachment #92126 -
        Attachment is obsolete: true

    
    

    
  
r=nelsonb for patch id 92753.  

Thanks for developing this patch, and for filtering out the nuisance lint 
warnings!  IMO, this patch shows good judgement in choosing which lint 
complaints to fix.

    
    

    
  
Nelson,

checked in riceman+bmo@mail.rit.edu's patch as requested to tip.

/cvsroot/mozilla/security/coreconf/nsinstall/pathsub.c,v  <--  pathsub.c
new revision: 1.4; previous revision: 1.3
done

/cvsroot/mozilla/security/nss/cmd/crmftest/testcrmf.c,v  <--  testcrmf.c
new revision: 1.3; previous revision: 1.2
done

/cvsroot/mozilla/security/nss/lib/crmf/crmfpop.c,v  <--  crmfpop.c
new revision: 1.2; previous revision: 1.1
done

/cvsroot/mozilla/security/nss/lib/cryptohi/seckey.c,v  <--  seckey.c
new revision: 1.19; previous revision: 1.18
done

/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cert.c,v  <--  pk11cert.c
new revision: 1.91; previous revision: 1.90
done

/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11pbe.c,v  <--  pk11pbe.c
new revision: 1.6; previous revision: 1.5
done

/cvsroot/mozilla/security/nss/lib/smime/smimeutil.c,v  <--  smimeutil.c
new revision: 1.9; previous revision: 1.8
done

/cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v  <--  pkcs11.c
new revision: 1.55; previous revision: 1.54
done

/cvsroot/mozilla/security/nss/lib/softoken/pkcs11c.c,v  <--  pkcs11c.c
new revision: 1.34; previous revision: 1.33
done

/cvsroot/mozilla/security/nss/lib/ssl/sslsock.c,v  <--  sslsock.c
new revision: 1.29; previous revision: 1.28
done

    
    

    
  
Fixed in NSS 3.6.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Target Milestone: 3.5 → 3.6

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: