Bug 364319 (CVE-2007-0008)

SSL2 client vulnerability in non-DEBUG builds

RESOLVED FIXED in 3.11.5

Status

NSS
Libraries
P1
critical
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: Nelson Bolyard (seldom reads bugmail))

Tracking

({fixed1.8.0.10, fixed1.8.1.2})

3.11.5
fixed1.8.0.10, fixed1.8.1.2
Bug Flags:
blocking1.8.1.2 +
blocking1.8.0.10 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical] [needs testscase])

Attachments

(1 attachment, 1 obsolete attachment)

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.
(Assignee)

Comment 1

11 years ago
Taking.  
Assignee: nobody → nelson
Priority: -- → P1
Target Milestone: --- → 3.11.5
(Assignee)

Comment 2

11 years ago
Created attachment 249088 [details] [diff] [review]
patch v1 (checked in)

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 3

11 years ago
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]

Comment 4

11 years ago
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.
(Assignee)

Comment 5

11 years ago
Created attachment 249159 [details] [diff] [review]
patch v2

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 6

11 years ago
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 7

11 years ago
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 8

11 years ago
Comment on attachment 249088 [details] [diff] [review]
patch v1 (checked in)

r+ = relyea
Attachment #249088 - Flags: review?(rrelyea) → review+
(Assignee)

Comment 9

11 years ago
patch v1 checked in.
(Assignee)

Updated

11 years ago
Attachment #249088 - Attachment description: patch v1 → patch v1 (checked in)
(Assignee)

Updated

11 years ago
Attachment #249159 - Attachment is obsolete: true
Attachment #249159 - Flags: review?(wtchang)
(Assignee)

Comment 10

11 years ago
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
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

10 years ago
Alias: CVE-2007-0008

Comment 11

10 years ago
Fixed on MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.
See bug 363070 comment 13.
Keywords: fixed1.8.0.10, fixed1.8.1.2

Updated

10 years ago
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.