337099 - Coverity Crash [@ PK11_ParamFromIV] Variable "iv" tracked as NULL was dereferenced.

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[timeless]

Comment 1

[Alexei Volkov]

Attachment: set len to 0 if iv is a prt to NULL or is zero length

Also check that len is not 0 since the len will be devided later in the "if"

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 222754 [details] [diff] [review]
set len to 0 if iv is a prt to NULL or is zero length

Alexei,

Please check the definition of mechanisms CKM_RC5_CBC and CKM_RC5_CBC_PAD.
IINM, they *require* a non-zero length IV (as all block ciphers should)
and so we should enforce that here rather than silently ignore it.

Also, please check the definition of mechanism CKM_RC5_ECB.  IINM, ECB ciphers
never require (or use) an IV, so checking for an IV for that mechanism 
(as the code now does) may be completely wrong.  

The RC5 mechanisms could be a "special case" among block cipher mechanisms,
using IVs in unusual ways (ways dissimilar to other block ciphers).  
But if it is not, we should correct our code to require IVs to be correct
(including absent, when necessary) for these block cipher mechanisms.

I'm withholding the review outcome pending that investigation.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 222754 [details] [diff] [review]
set len to 0 if iv is a prt to NULL or is zero length

It's clear that NONE of the cases in this switch do any sanity checkin on their inputs.  None of them require non-zero-length ivs for block cipher mechanisms. So I won't ask you to fix that for this coverity bug.

Comment 4

[Alexei Volkov]

tip
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11mech.c,v  <--  pk11mech.c
new revision: 1.5; previous revision: 1.4

3.11 branch
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11mech.c,v  <--  pk11mech.c
new revision: 1.4.2.1; previous revision: 1.4