337098 - Coverity crash [@ PK11_FreeSymKey - pk11_KeyExchange] confused logic

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[timeless]

Event var_compare_op: Added "tekTarget" due to comparison "tekTarget == 0"
Event var_deref_model: Variable "tekTarget" tracked as NULL was passed to a function that dereferences it. [model]
Also see events: [var_deref_model]
At conditional (1): "tekTarget == 0" taking true path

228  		if (tekTarget == NULL) PK11_FreeSymKey(tekTarget);

Afaik the code should be using != or just (tekTarget) et al.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

The URL given in the original bug report was for the wrong file.
The code in question is old FORTEZZA code.  
The entire error path is bogus. But rather than fixing it, the entire 
block of FORTEZZA KEA code in this file should be eliminated.  

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

taking

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Attachment: remove KEA support from pk11_KeyExchange

Bob, does this seem right to you?
Is there any other change in softoken that must be made to 
accompany this?

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Adding requetsed reviewer to cc list.

Comment 5

[Wan-Teh Chang]

Comment on attachment 222468 [details] [diff] [review]
remove KEA support from pk11_KeyExchange

r=wtc.  Alternately, change all the == NULL tests in
the error path to != NULL tests.  I agree it's better
to remove KEA support.

Comment 6

[Robert Relyea]

Comment on attachment 222468 [details] [diff] [review]
remove KEA support from pk11_KeyExchange

Removing KEA is the right way to go. We've already removed much of the fortezza code. This is basically dead code now, so removing it should be ok.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Thanks for the reviews.
Checking in pk11wrap/pk11kea.c; new revision: 1.11; previous revision: 1.10
Checking in pk11wrap/pk11kea.c; new revision: 1.9.28.1; previous revision: 1.9

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

CID 320