Closed Bug 364319 (CVE-2007-0008) Opened 15 years ago Closed 15 years ago

SSL2 client vulnerability in non-DEBUG builds

Categories

(NSS :: Libraries, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
3.11.5

People

(Reporter: nelson, Assigned: nelson)

Details

(Keywords: fixed1.8.0.10, fixed1.8.1.2, Whiteboard: [sg:critical] [needs testscase])

Attachments

(1 file, 1 obsolete file)

This vulnerability was reported to security@mozilla.org by email from 
vendor-disclosure@idefense.com, bearing the subject IDEF1973.  It says:
"This vulnerability was reported to iDefense by regenrecht."

An SSL server cert with an RSA public key that is too small to encrypt
the entire SSL2 "Master Secret" will cause optimized (not DEBUG) NSS 
builds to crash, and/or execute attacker-supplied code (on some platforms).
Disabling SSL2 is a complete defense.  Products with SSL2 disabled are 
believed to be invulnerable.

The bug has been present in NSS since at least NSS 3.4 and probably longer.
Function ssl_FormatSSL2Block starts with an assertion that the modulus is
long enough to properly encode the data to be encrypted, but that only has
effect in DEBUG builds.  In non-DEBUG builds, when the modulus is too short,
the code overruns the end of the too-short buffer.  The fix is to detect 
too-short modulii in all builds, not just DEBUG builds.  This probably 
should not be an ASSERT at all, but just a run time check.
Taking.  
Assignee: nobody → nelson
Priority: -- → P1
Target Milestone: --- → 3.11.5
Bob, Wan-Teh, please review.

Others: remember that FF2 already has SSL2 disabled, and therefore is immune.
Attachment #249088 - Flags: superreview?(wtchang)
Attachment #249088 - Flags: review?(rrelyea)
Comment on attachment 249088 [details] [diff] [review]
patch v1 (checked in)

r=wtc.
Attachment #249088 - Flags: superreview?(wtchang) → superreview+
Flags: blocking1.8.1.2+
Flags: blocking1.8.0.10+
Whiteboard: [sg:critical]
I reviewed all the other assertions in sslcon.c.
Let's do something about this closely related assertion in sslcon.c:

    PORT_Assert((modulusLen - rek.len) > 12);

Note that > 12 is the same as >= 13.  13 is greater than
(3 + RSA_BLOCK_MIN_PAD_LEN) by 2.
Attached patch patch v2 (obsolete) — Splinter Review
Incorporates Wan-Teh's suggestion (I think).

Wan-Teh, is this what you hand in mind?
If so, please mark patch v1 obsolete and ask Bob to review this one.
Attachment #249159 - Flags: review?(wtchang)
Comment on attachment 249159 [details] [diff] [review]
patch v2

Nelson, if you agree that the "> 12" in that assertion is wrong
and should be ">= 11", we should simply remove that assertion.
The check you added in this patch is redundant; it's already
performed in the ssl_FormatSSL2Block call above.  Thanks.
Comment on attachment 249159 [details] [diff] [review]
patch v2

I have an idea for how to deal with the assertion in
ssl2_ClientSetupSessionCypher: move the following code,
which sets up the padding for version 2 rollback detection,
to ssl_FormatSSL2Block.  This requires passing 'ss' or
a boolean flag as a new argument to ssl_FormatSSL2Block
though.

    /* Set up the padding for version 2 rollback detection. */
    /* XXX We should really use defines here */
    if (ss->opt.enableSSL3 || ss->opt.enableTLS) {
        PORT_Assert((modulusLen - rek.len) > 12);
        PORT_Memset(eblock + modulusLen - rek.len - 8 - 1, 0x03, 8);
    }

If this code is in ssl_FormatSSL2Block, it'll be clear
that the assertion is redundant and can be simply removed.
Comment on attachment 249088 [details] [diff] [review]
patch v1 (checked in)

r+ = relyea
Attachment #249088 - Flags: review?(rrelyea) → review+
patch v1 checked in.
Attachment #249088 - Attachment description: patch v1 → patch v1 (checked in)
Attachment #249159 - Attachment is obsolete: true
Attachment #249159 - Flags: review?(wtchang)
The checkin was

> Improve checking of received SSL2 records.
> Bug 364319, bug 364323. r=rrelyea, wtchang

on trunk:
Checking in sslcon.c; new revision: 1.35; previous revision: 1.34

On branch:
Checking in sslcon.c; new revision: 1.28.2.7; previous revision: 1.28.2.6
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Alias: CVE-2007-0008
Fixed on MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.
See bug 363070 comment 13.
Whiteboard: [sg:critical] → [sg:critical] [needs testscase]
Group: security
You need to log in before you can comment on or make changes to this bug.