Bug 499444 - Infinite recursion when encoding a NSS enveloped/digested data
Summary: Infinite recursion when encoding a NSS enveloped/digested data
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: nss
Version: 14
Hardware: All
OS: Linux
medium
high
Target Milestone: ---
Assignee: Kai Engert (:kaie) (inactive account)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 668882
TreeView+ depends on / blocked
 
Reported: 2009-05-06 17:15 UTC by Miloslav Trmač
Modified: 2011-03-14 17:49 UTC (History)
4 users (show)

Fixed In Version: nss-softokn-3.12.9-7.fc15
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 668882 (view as bug list)
Environment:
Last Closed: 2011-03-14 17:49:52 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Reproducer, see comment#0. (6.71 KB, text/plain)
2009-05-06 20:11 UTC, Miloslav Trmač
no flags Details
sample (incorrect?) output (1.45 KB, application/octet-stream)
2011-03-11 12:16 UTC, Miloslav Trmač
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Mozilla Foundation 491918 0 None None None Never

Description Miloslav Trmač 2009-05-06 17:15:51 UTC
Version:
nss-3.12.2.0-5.fc10.x86_64, and a locally rebuilt nss-3.23.3-3.

Attempting to encode a CMS message that contains enveloped (digested (data)) causes infinite recursion and stack overflow.

To reproduce, compile the attached file with
   gcc $(pkg-config --cflags --libs nss glib-2.0) repro.c
and run
   ./a.out a/path/to/a/certificate.pem > x
This will correctly create an enveloped message.

If you redefine DIGESTED in the file to "1" and recompile,
   ./a.out a/path/to/a/certificate.pem > x
will crash with a stack overflow.

AFAICS this is going on:
* When secasn1e.c finishes encoding the contentType field of NSSCMSEncryptedContentInfoTemplate, sec_asn1e_next_in_sequence() gets called.
* ...next_in_sequence does the "after" field notification before changing any ASN1 state.
* The "after" notification ends up in nss_cms_before_data(), which creates a completely new encoding context for the "child" digested_data, and starts encoding it.
* Encoding the "header" of digested_data (algorithm ID, nested raw data OID) requires more than 16 bytes, so nss_cms_encoder_work_data() will get the encrypted data and call SEC_ASN1EncoderUpdate() with it in the enveloped_data context.
* As noted above, the ASN1 context was not changed before calling the "after" field notification of contentType, so SEC_ASN1EncoderUpdate() again thinks the "contentType" field was just finished encoding, calls the "after" notification again, and the whole process (recursively) repeats.

See also #499440, a different bug related to the calling SEC_ASN1EncoderUpdate() from within a notifier called sec_asn1e_next_in_sequence ().

Comment 1 Miloslav Trmač 2009-05-06 20:11:04 UTC
Created attachment 342711 [details]
Reproducer, see comment#0.

Comment 2 Kai Engert (:kaie) (inactive account) 2009-05-07 19:47:24 UTC
Library functionality bugs should get reported upstream.
I've forwarded your bug reported 1:1 at 
https://bugzilla.mozilla.org/show_bug.cgi?id=491918

Comment 3 Fedora Update System 2011-02-11 22:35:34 UTC
nss-3.12.9-10.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/nss-3.12.9-10.fc15

Comment 4 Fedora Update System 2011-03-03 03:22:56 UTC
nss-softokn-3.12.9-7.fc15, nss-3.12.9-13.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 5 Miloslav Trmač 2011-03-03 06:20:28 UTC
I'm afraid I still see a stack overflow - if the defines section contains

#define ENVELOPED 1
#define DIGESTED 1
#define INLINE_DATA 0

(the original problem description was incorrect about the macros, sorry about that.)

Comment 6 Miloslav Trmač 2011-03-03 06:21:06 UTC
(In reply to comment #5)
> I'm afraid I still see a stack overflow 
nss-3.12.9-8.fc14.x86_64
nss-softokn-3.12.9-5.fc14.x86_64

Comment 7 Nalin Dahyabhai 2011-03-04 16:30:07 UTC
(In reply to comment #5)
> I'm afraid I still see a stack overflow - if the defines section contains
> 
> #define ENVELOPED 1
> #define DIGESTED 1
> #define INLINE_DATA 0
> 
> (the original problem description was incorrect about the macros, sorry about
> that.)

The testing updates are for f15, but I don't see f15's allow-content-types-beyond-smime or nss-recurse patches in the f14 branch.  (If I apply them manually, though, the reproducer's happy.)

Comment 8 Elio Maldonado Batiz 2011-03-04 17:09:27 UTC
(In reply to comment #7)
> The testing updates are for f15, but I don't see f15's
> allow-content-types-beyond-smime or nss-recurse patches in the f14 branch.  (If
> I apply them manually, though, the reproducer's happy.)

Yes, I didn't include the patches in the stable f14 branch and opted instead to wait and pick them up when we do the update to 3.12.10. f15 got them as they where in rawhide already. Would it be possible to get some simple reproducer/verifier test attached to the bug?

Comment 9 Miloslav Trmač 2011-03-11 12:16:50 UTC
Created attachment 483700 [details]
sample (incorrect?) output

(In reply to comment #7)
> The testing updates are for f15, but I don't see f15's
> allow-content-types-beyond-smime or nss-recurse patches in the f14 branch.  (If
> I apply them manually, though, the reproducer's happy.)

Right, this does not hang on F15.  Reading the F14 changelog, I thought that this problem was fixed in F14 as well, sorry about that.


However, the F15 output does not seem to be correct.  Attached is an example - looking at it through (openssl asn1parse -inform der -i < x), where I'd expect one "octet string" containing the encrypted representation of DigestedData, there are _6_ separate octet strings.  When I (manually) concatenate them and decrypt them, I get a valid DigestedData structure - but my reading of both PKCS#7 and RFC 3852 suggests that there should be only one octet string.

Comment 10 Nalin Dahyabhai 2011-03-14 17:14:45 UTC
That should be valid if the octet string is tagged as constructed.  It surprised me when I first saw it happening, but it's a consequence of the contained data item being itself fed into the encoder in chunks.

Comment 11 Miloslav Trmač 2011-03-14 17:49:52 UTC
(In reply to comment #10)
> That should be valid if the octet string is tagged as constructed.  It
> surprised me when I first saw it happening, but it's a consequence of the
> contained data item being itself fed into the encoder in chunks.

Ah, so it is just my ignorance of ASN.1.  I don't understand why is the "constructed octet string" should be tagged as "context-specific 0", but that's probably my ignorance again.

Thanks for the explanation; the infinite recursion is definitely fixed in F15, so this bug can be closed.


Note You need to log in before you can comment on or make changes to this bug.