337104 - Coverity OOM Crash [@ ssl2_QualifyCypherSpecs][@ ssl2_ChooseSessionCypher] Variable "ms" tracked as NULL was dereferenced.

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[timeless]

The code assumes that ss->sizeCipherSpecs implies ss->cipherSpecs or that ssl2_ConstructCipherSpecs can safely initialize ss->ciperSpecs

which is false, the function can clearly return SECFailure.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Timeless, this bug summary claims to be about two functions whose names start
wtih ssl2_, none of which are in the file ssl3con.c.
But the URL given for this bug,
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/ssl3con.c&rev=1.88&mark=1769-1770,1773,1775,1776,1777,1830-1831,1834-1835,1838-1841,1843,1858,1859,1860#1769
is for a function named ssl3_CompressMACEncryptRecord in ssl3con.c

So this begs several questions.  
which of these is this bug really supposed to be about?
Is there another bug for the other one?

I will proceed assuming that this bug is really about the functions named 
in the bug summary (unless that proves to be a dead end).
If you need to file another bug about the URL given above, please do so,
but be sure to explain what's wrong with that URL.

Comment 2

[timeless]

sorry, i'm currently in amsterdam, it's quite possible i lost a bug while trying to file these. even if i weren't, i wouldn't really recall much more about these bugs, although i suppose i could ask coverity what bug# i claimed for the functions in the original summary. i'll see about maybe doing that on tuesday at the earliest :(.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Eliminating unrelated bug URL, and replacing it with these two URLs
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/sslcon.c&rev=1.30&mark=1767-1769,1773,1775#1752
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/sslcon.c&rev=1.30&mark=1828-1830,1857,1866#1827
which I derived from the bug summary.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

OK, it'a apparent now that the old URL simply had the wrong file name in it.
The new URL is the right one.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch for bug 337104 and bug 337105, v1

This patch fixes two bugs (which were both about the same source file
and even about the same function).

Comment 6

[Alexei Volkov]

Comment on attachment 222570 [details] [diff] [review]
patch for bug 337104 and bug 337105, v1

r=alexei

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Checking in sslcon.c; new revision: 1.31;     previous revision: 1.30
Checking in sslcon.c; new revision: 1.28.2.3; previous revision: 1.28.2.2

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Also CID 444