894370 - (CVE-2013-1739) Assertion failure: inputLen >= spec->mac_size, at c:/work/mozilla/builds/beta/mozilla/security/nss/lib/ssl/ssl3con.c:2057

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Carsten Book [:Tomcat]]

Attachment: stack

Happens in beta and nightly debug builds on all platforms according to bughunter

Assertion failure: inputLen >= spec->mac_size, at c:/work/mozilla/builds/beta/mozilla/security/nss/lib/ssl/ssl3con.c:2057

Will try to generate a testcase

Steps to repoduce is to load https://sc.lcsd.gov.hk/uniS/webcat.hkpl.gov.hk/lib/item;jsessionid=1414DB917107CA0666E4352692442460?id=chamo:3207909&theme=WEB&copies-sort=3%2Basc in a debug build and nearly crash on load

Comment 1

[Carsten Book [:Tomcat]]

note, there seems to be a problem with this site. While trying to use wget to create the testcase i got:

Connecting to sc.lcsd.gov.hk (sc.lcsd.gov.hk)|202.53.141.71|:443... connected.
GnuTLS: Key usage violation in certificate has been detected.
Unable to establish SSL connection.

Comment 2

[Kai Engert (:KaiE:)]

Seems to refer to:

ssl3_ComputeRecordMACConstantTime(
...
    PORT_Assert(inputLen >= spec->mac_size);


I cannnot reproduce the assertion using a debug build of tstclnt and settings that are probably close to what firefox beta with NSS 3.15 uses:

tstclnt -d . -p 443 -h sc.lcsd.gov.hk -o -T -v -V ssl3:tls1.1 -f -O < req.txt 

cat req.txt:
GET / HTTP/1.1
Host: sc.lcsd.gov.hk

Comment 3

[Kai Engert (:KaiE:)]

(In reply to Carsten Book [:Tomcat] from comment #1)
> note, there seems to be a problem with this site. While trying to use wget
> to create the testcase i got:
> 
> Connecting to sc.lcsd.gov.hk (sc.lcsd.gov.hk)|202.53.141.71|:443...
> connected.
> GnuTLS: Key usage violation in certificate has been detected.
> Unable to establish SSL connection.


Works for me, wget reports a 404, on Fedora 19 and also another distribution.

Your wget uses GnuTLS. Which Linux distribution do you use?

Comment 4

[Carsten Book [:Tomcat]]

(In reply to Kai Engert (:kaie) from comment #3)
> Works for me, wget reports a 404, on Fedora 19 and also another distribution.
> 
> Your wget uses GnuTLS. Which Linux distribution do you use?

This was with the Version from cygwin

Comment 5

[Adam Langley]

I cannot reproduce the crash from the given URL, but I can see the bug from the code because it's my fault.

The code checks the size of the input, but it assumes that decryption doesn't alter the size of the data. This is incorrect for case of a block cipher in NSS - the decryption may result in zero bytes of output if the input isn't a multiple of the block size.

The issue is around this line:

  /* decrypt from cText buf to plaintext. */
      rv = crSpec->decode(
  »·······crSpec->decodeContext, plaintext->buf, (int *)&plaintext->len,
  »·······plaintext->space, cText->buf->buf + ivLen, cText->buf->len - ivLen);

plaintext->len can end up zero.

However, I cannot see a way to exploit this (although others are better at that sort of thing than I). Both ssl_RemoveSSLv3CBCPadding and ssl_RemoveTLSCBCPadding will reject the length immediately so the good flag will be cleared.

The danger lies in ssl3_ComputeRecordMACConstantTime and ssl_CBCExtractMAC. Dealing with the latter first, because it's shorter, the main loop will exit immediately because originalLength is zero and |i < originalLength| will be false.

In ssl_ComputeRecordMACConstantTime, |inputLen| and |originalLen| will be zero (triggering the assertion in debug mode). |recordLength| will underflow, but it's only used to update a couple of bytes |header| and it doesn't matter that they'll be nonsense.

There there are two paths: ssl_ComputeRecordMAC in the fallback case (when NSS and libssl have version skew) and the constant time MAC. For the former, it gets a negative length and passes that to PK11_DigestOp and the like. This will either do nothing, or will try to hash all of memory and crash.

For the constant time code we have to go to lib/freebl/hmacct.c. Here, maxMACBytes will underflow but the calculation in numBlocks will push it positive again. It'll hash a little uninitialised data, but it doesn't go crazy.

Back in ssl3_HandleRecord, plaintext->len will be updated and will underflow, which is very dangerous, but the good flag must be false and so the function will send a fatal alert and return.

To test I updated a server to trigger this problem and commented the assertions to see what happened with a debug build of Chrome and it resulted in a connection error without crashing.



To address that I think the decryption failure should be handled synchronously. The code is security-sensitive-constant-time but the length of the ciphertext is public information and so it's ok to have different behaviour based on that. Additionally, some decryptions may not set the output length at all if they return an error.

I'll upload a patch to that effect in a sec.

Comment 6

[Adam Langley]

Attachment: patch

Patch. Not yet tested at all so no r? flags.

Comment 7

[Andrew McCreight [:mccr8]]

I'm marking this sec-moderate because from comment 5 this doesn't sound bad.  Feel free to adjust up or down as desired.

Comment 8

[Adam Langley]

Attachment: patch

This is the patch that I ended up committing to Chromium for this issue.

Comment 9

[Wan-Teh Chang]

Comment on attachment 785022 [details] [diff] [review]
patch

r=wtc. Patch checked in on the NSS trunk:
https://hg.mozilla.org/projects/nss/rev/56436aa3463f

Comment 10

[Wan-Teh Chang]

Marked the bug fixed.

The code in question was added in NSS 3.14.3 to fix bug 822365.

Comment 11

[Daniel Veditz [:dveditz]]

This appears fixed in Firefox nightly and aurora (Firefox 26, 27), but not beta or release (Firefox 24, 25).

Comment 12

[Daniel Veditz [:dveditz]]

Assigning CVE-2013-1739 to this issue.

Comment 13

[Daniel Veditz [:dveditz]]

We should fix this in Firefox 25 and the corresponding Firefox ESR-24 by upgrading from NSS 3.15.1 to 3.15.2

It is not severe enough to be worth upgrading the last ESR 17 release from NSS 3.14.3

Comment 14

[Kai Engert (:KaiE:)]

Adding Elio for picking up the patch.

Comment 15

[Alex Keybl [:akeybl]]

NSS for mozilla-beta was updated to 3.15.2 in https://hg.mozilla.org/releases/mozilla-beta/rev/ee766b7c606e (bug 921090).

Comment 16

[Ryan VanderMeulen [:RyanVM]]

(In reply to Daniel Veditz [:dveditz] from comment #13)
> It is not severe enough to be worth upgrading the last ESR 17 release from
> NSS 3.14.3

Same for b2g18?

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Who is doing the ESR24 patch for this since it is tracking+ for the next ESR24 release in the next two weeks?

Comment 18

[Kai Engert (:KaiE:)]

Al, this bug has been fixed in NSS 3.15.2

It seems the ESR24 branch has already been updated to use NSS 3.15.2 in the meantime:
http://hg.mozilla.org/releases/mozilla-esr24/file/default/security/nss/TAG-INFO

Comment 19

[Al Billings [:abillings - ex-MoCo]]

Changing flags to mark ESR24 as fixed.

Comment 20

[Daniel Veditz [:dveditz]]

NSS 3.14.4 was created for ESR 17 so this is fixed there now, too.