Bug 1211586, NSPR_4_10_10_RC1 and NSS_3_19_4_RC0, a=sledru
authorKai Engert <kaie@kuix.de>
Fri, 16 Oct 2015 22:57:29 +0200
changeset 289573 170d29280d87
parent 289572 d7c8d0af1b08
child 289574 ed67ac61d1c0
push id5195
push userkaie@kuix.de
push date2015-10-16 20:57 +0000
treeherdermozilla-beta@170d29280d87 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssledru
bugs1211586
milestone42.0
Bug 1211586, NSPR_4_10_10_RC1 and NSS_3_19_4_RC0, a=sledru
nsprpub/TAG-INFO
nsprpub/config/prdepend.h
nsprpub/configure
nsprpub/configure.in
nsprpub/lib/ds/plarena.c
nsprpub/lib/ds/plarena.h
nsprpub/pr/include/md/_linux.cfg
nsprpub/pr/include/prinit.h
nsprpub/pr/tests/vercheck.c
security/nss/TAG-INFO
security/nss/coreconf/coreconf.dep
security/nss/lib/nss/nss.h
security/nss/lib/softoken/softkver.h
security/nss/lib/util/nssutil.h
security/nss/lib/util/secasn1d.c
--- a/nsprpub/TAG-INFO
+++ b/nsprpub/TAG-INFO
@@ -1,1 +1,1 @@
-NSPR_4_10_9_RTM
+NSPR_4_10_10_RC1
--- a/nsprpub/config/prdepend.h
+++ b/nsprpub/config/prdepend.h
@@ -5,9 +5,8 @@
 
 /*
  * A dummy header file that is a dependency for all the object files.
  * Used to force a full recompilation of NSPR in Mozilla's Tinderbox
  * depend builds.  See comments in rules.mk.
  */
 
 #error "Do not include this header file."
-
--- a/nsprpub/configure
+++ b/nsprpub/configure
@@ -2484,17 +2484,17 @@ case $target_os in *\ *) target_os=`echo
 # will get canonicalized.
 test -n "$target_alias" &&
   test "$program_prefix$program_suffix$program_transform_name" = \
     NONENONEs,x,x, &&
   program_prefix=${target_alias}-
 
 MOD_MAJOR_VERSION=4
 MOD_MINOR_VERSION=10
-MOD_PATCH_VERSION=9
+MOD_PATCH_VERSION=10
 NSPR_MODNAME=nspr20
 _HAVE_PTHREADS=
 USE_PTHREADS=
 USE_USER_PTHREADS=
 USE_NSPR_THREADS=
 USE_N32=
 USE_X32=
 USE_64=
--- a/nsprpub/configure.in
+++ b/nsprpub/configure.in
@@ -11,17 +11,17 @@ AC_CONFIG_SRCDIR([pr/include/nspr.h])
 AC_CONFIG_AUX_DIR(${srcdir}/build/autoconf)
 AC_CANONICAL_TARGET
 
 dnl ========================================================
 dnl = Defaults
 dnl ========================================================
 MOD_MAJOR_VERSION=4
 MOD_MINOR_VERSION=10
-MOD_PATCH_VERSION=9
+MOD_PATCH_VERSION=10
 NSPR_MODNAME=nspr20
 _HAVE_PTHREADS=
 USE_PTHREADS=
 USE_USER_PTHREADS=
 USE_NSPR_THREADS=
 USE_N32=
 USE_X32=
 USE_64=
--- a/nsprpub/lib/ds/plarena.c
+++ b/nsprpub/lib/ds/plarena.c
@@ -88,16 +88,19 @@ PR_IMPLEMENT(void) PL_InitArenaPool(
         align = PL_ARENA_DEFAULT_ALIGN;
 
     if (align < sizeof(pmasks)/sizeof(pmasks[0]))
         pool->mask = pmasks[align];
     else
         pool->mask = PR_BITMASK(PR_CeilingLog2(align));
 
     pool->first.next = NULL;
+    /* Set all three addresses in pool->first to the same dummy value.
+     * These addresses are only compared with each other, but never
+     * dereferenced. */
     pool->first.base = pool->first.avail = pool->first.limit =
         (PRUword)PL_ARENA_ALIGN(pool, &pool->first + 1);
     pool->current = &pool->first;
     /*
      * Compute the net size so that each arena's gross size is |size|.
      * sizeof(PLArena) + pool->mask is the header and alignment slop
      * that PL_ArenaAllocate adds to the net size.
      */
@@ -139,20 +142,24 @@ PR_IMPLEMENT(void) PL_InitArenaPool(
 ** See also: bugzilla: 45343.
 **
 */
 
 PR_IMPLEMENT(void *) PL_ArenaAllocate(PLArenaPool *pool, PRUint32 nb)
 {
     PLArena *a;   
     char *rp;     /* returned pointer */
+    PRUint32 nbOld;
 
     PR_ASSERT((nb & pool->mask) == 0);
     
+    nbOld = nb;
     nb = (PRUword)PL_ARENA_ALIGN(pool, nb); /* force alignment */
+    if (nb < nbOld)
+        return NULL;
 
     /* attempt to allocate from arenas at pool->current */
     {
         a = pool->current;
         do {
             if ( nb <= a->limit - a->avail )  {
                 pool->current = a;
                 rp = (char *)a->avail;
@@ -203,16 +210,17 @@ PR_IMPLEMENT(void *) PL_ArenaAllocate(PL
             a = (PLArena*)PR_MALLOC(sz);
         }
         if ( NULL != a )  {
             a->limit = (PRUword)a + sz;
             a->base = a->avail = (PRUword)PL_ARENA_ALIGN(pool, a + 1);
             PL_MAKE_MEM_NOACCESS((void*)a->avail, a->limit - a->avail);
             rp = (char *)a->avail;
             a->avail += nb;
+            PR_ASSERT(a->avail <= a->limit);
             /* the newly allocated arena is linked after pool->current 
             *  and becomes pool->current */
             a->next = pool->current->next;
             pool->current->next = a;
             pool->current = a;
             if ( NULL == pool->first.next )
                 pool->first.next = a;
             PL_COUNT_ARENA(pool,++);
@@ -225,16 +233,18 @@ PR_IMPLEMENT(void *) PL_ArenaAllocate(PL
     return(NULL);
 } /* --- end PL_ArenaAllocate() --- */
 
 PR_IMPLEMENT(void *) PL_ArenaGrow(
     PLArenaPool *pool, void *p, PRUint32 size, PRUint32 incr)
 {
     void *newp;
 
+    if (PR_UINT32_MAX - size < incr)
+        return NULL;
     PL_ARENA_ALLOCATE(newp, pool, size + incr);
     if (newp)
         memcpy(newp, p, size);
     return newp;
 }
 
 static void ClearArenaList(PLArena *a, PRInt32 pattern)
 {
--- a/nsprpub/lib/ds/plarena.h
+++ b/nsprpub/lib/ds/plarena.h
@@ -132,44 +132,49 @@ void __asan_unpoison_memory_region(void 
         PL_InitArenaPool(pool, name, size, PL_ARENA_CONST_ALIGN_MASK + 1)
 #else
 #define PL_ARENA_ALIGN(pool, n) (((PRUword)(n) + (pool)->mask) & ~(pool)->mask)
 #endif
 
 #define PL_ARENA_ALLOCATE(p, pool, nb) \
     PR_BEGIN_MACRO \
         PLArena *_a = (pool)->current; \
-        PRUint32 _nb = PL_ARENA_ALIGN(pool, nb); \
+        PRUint32 _nb = PL_ARENA_ALIGN(pool, (PRUint32)nb); \
         PRUword _p = _a->avail; \
-        PRUword _q = _p + _nb; \
-        if (_q > _a->limit) { \
+        if (_nb < (PRUint32)nb) { \
+            _p = 0; \
+        } else if (_nb > (_a->limit - _a->avail)) { \
             _p = (PRUword)PL_ArenaAllocate(pool, _nb); \
         } else { \
-            _a->avail = _q; \
+            _a->avail += _nb; \
         } \
         p = (void *)_p; \
-        PL_MAKE_MEM_UNDEFINED(p, nb); \
-        PL_ArenaCountAllocation(pool, nb); \
+        if (p) { \
+            PL_MAKE_MEM_UNDEFINED(p, (PRUint32)nb); \
+            PL_ArenaCountAllocation(pool, (PRUint32)nb); \
+        } \
     PR_END_MACRO
 
 #define PL_ARENA_GROW(p, pool, size, incr) \
     PR_BEGIN_MACRO \
         PLArena *_a = (pool)->current; \
-        PRUint32 _incr = PL_ARENA_ALIGN(pool, incr); \
-        PRUword _p = _a->avail; \
-        PRUword _q = _p + _incr; \
-        if (_p == (PRUword)(p) + PL_ARENA_ALIGN(pool, size) && \
-            _q <= _a->limit) { \
-            PL_MAKE_MEM_UNDEFINED((unsigned char *)(p) + size, incr); \
-            _a->avail = _q; \
-            PL_ArenaCountInplaceGrowth(pool, size, incr); \
+        PRUint32 _incr = PL_ARENA_ALIGN(pool, (PRUint32)incr); \
+        if (_incr < (PRUint32)incr) { \
+            p = NULL; \
+        } else if (_a->avail == (PRUword)(p) + PL_ARENA_ALIGN(pool, size) && \
+            _incr <= (_a->limit - _a->avail)) { \
+            PL_MAKE_MEM_UNDEFINED((unsigned char *)(p) + size, (PRUint32)incr); \
+            _a->avail += _incr; \
+            PL_ArenaCountInplaceGrowth(pool, size, (PRUint32)incr); \
         } else { \
-            p = PL_ArenaGrow(pool, p, size, incr); \
+            p = PL_ArenaGrow(pool, p, size, (PRUint32)incr); \
         } \
-        PL_ArenaCountGrowth(pool, size, incr); \
+        if (p) {\
+            PL_ArenaCountGrowth(pool, size, (PRUint32)incr); \
+        } \
     PR_END_MACRO
 
 #define PL_ARENA_MARK(pool) ((void *) (pool)->current->avail)
 #define PR_UPTRDIFF(p,q) ((PRUword)(p) - (PRUword)(q))
 
 #define PL_CLEAR_UNUSED_PATTERN(a, pattern) \
     PR_BEGIN_MACRO \
         PR_ASSERT((a)->avail <= (a)->limit); \
--- a/nsprpub/pr/include/md/_linux.cfg
+++ b/nsprpub/pr/include/md/_linux.cfg
@@ -503,17 +503,17 @@
 #undef  IS_LITTLE_ENDIAN
 #elif defined(__MIPSEL__)
 #define IS_LITTLE_ENDIAN 1
 #undef  IS_BIG_ENDIAN
 #else
 #error "Unknown MIPS endianness."
 #endif
 
-#ifdef _ABI64
+#if _MIPS_SIM == _ABI64
 
 #define IS_64
 
 #define PR_BYTES_PER_BYTE   1
 #define PR_BYTES_PER_SHORT  2
 #define PR_BYTES_PER_INT    4
 #define PR_BYTES_PER_INT64  8
 #define PR_BYTES_PER_LONG   8
--- a/nsprpub/pr/include/prinit.h
+++ b/nsprpub/pr/include/prinit.h
@@ -26,20 +26,20 @@ PR_BEGIN_EXTERN_C
 /*
 ** NSPR's version is used to determine the likelihood that the version you
 ** used to build your component is anywhere close to being compatible with
 ** what is in the underlying library.
 **
 ** The format of the version string is
 **     "<major version>.<minor version>[.<patch level>] [<Beta>]"
 */
-#define PR_VERSION  "4.10.9"
+#define PR_VERSION  "4.10.10"
 #define PR_VMAJOR   4
 #define PR_VMINOR   10
-#define PR_VPATCH   9
+#define PR_VPATCH   10
 #define PR_BETA     PR_FALSE
 
 /*
 ** PRVersionCheck
 **
 ** The basic signature of the function that is called to provide version
 ** checking. The result will be a boolean that indicates the likelihood
 ** that the underling library will perform as the caller expects.
--- a/nsprpub/pr/tests/vercheck.c
+++ b/nsprpub/pr/tests/vercheck.c
@@ -15,52 +15,52 @@
  */
 
 #include "prinit.h"
 
 #include <stdio.h>
 #include <stdlib.h>
 
 /*
- * This release (4.10.7) is backward compatible with the
+ * This release (4.10.10) is backward compatible with the
  * 4.0.x, 4.1.x, 4.2.x, 4.3.x, 4.4.x, 4.5.x, 4.6.x, 4.7.x,
  * 4.8.x, 4.9.x, 4.10, 4.10.1, 4.10.2, 4.10.3, 4.10.4,
- * 4.10.5, 4.10.6, 4.10.7 and 4.10.8 releases.
+ * 4.10.5, 4.10.6, 4.10.7, 4.10.8, 4.10.9 releases.
  * It, of course, is compatible with itself.
  */
 static char *compatible_version[] = {
     "4.0", "4.0.1", "4.1", "4.1.1", "4.1.2", "4.1.3",
     "4.2", "4.2.1", "4.2.2", "4.3", "4.4", "4.4.1",
     "4.5", "4.5.1",
     "4.6", "4.6.1", "4.6.2", "4.6.3", "4.6.4", "4.6.5",
     "4.6.6", "4.6.7", "4.6.8",
     "4.7", "4.7.1", "4.7.2", "4.7.3", "4.7.4", "4.7.5",
     "4.7.6",
     "4.8", "4.8.1", "4.8.2", "4.8.3", "4.8.4", "4.8.5",
     "4.8.6", "4.8.7", "4.8.8", "4.8.9",
     "4.9", "4.9.1", "4.9.2", "4.9.3", "4.9.4", "4.9.5",
     "4.9.6",
     "4.10", "4.10.1", "4.10.2", "4.10.3", "4.10.4",
-    "4.10.5", "4.10.6", "4.10.7", "4.10.8",
+    "4.10.5", "4.10.6", "4.10.7", "4.10.8", "4.10.9",
     PR_VERSION
 };
 
 /*
  * This release is not backward compatible with the old
  * NSPR 2.1 and 3.x releases.
  *
  * Any release is incompatible with future releases and
  * patches.
  */
 static char *incompatible_version[] = {
     "2.1 19980529",
     "3.0", "3.0.1",
     "3.1", "3.1.1", "3.1.2", "3.1.3",
     "3.5", "3.5.1",
-    "4.10.10",
+    "4.10.11",
     "4.11", "4.11.1",
     "10.0", "11.1", "12.14.20"
 };
 
 int main(int argc, char **argv)
 {
     int idx;
     int num_compatible = sizeof(compatible_version) / sizeof(char *);
--- a/security/nss/TAG-INFO
+++ b/security/nss/TAG-INFO
@@ -1,1 +1,1 @@
-NSS_3_19_3_RTM
+NSS_3_19_4_RC0
--- a/security/nss/coreconf/coreconf.dep
+++ b/security/nss/coreconf/coreconf.dep
@@ -5,8 +5,9 @@
 
 /*
  * A dummy header file that is a dependency for all the object files.
  * Used to force a full recompilation of NSS in Mozilla's Tinderbox
  * depend builds.  See comments in rules.mk.
  */
 
 #error "Do not include this header file."
+
--- a/security/nss/lib/nss/nss.h
+++ b/security/nss/lib/nss/nss.h
@@ -28,20 +28,20 @@
 
 /*
  * NSS's major version, minor version, patch level, build number, and whether
  * this is a beta release.
  *
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>[.<build number>]][ <ECC>][ <Beta>]"
  */
-#define NSS_VERSION  "3.19.3" _NSS_ECC_STRING _NSS_CUSTOMIZED
+#define NSS_VERSION  "3.19.4" _NSS_ECC_STRING _NSS_CUSTOMIZED
 #define NSS_VMAJOR   3
 #define NSS_VMINOR   19
-#define NSS_VPATCH   3
+#define NSS_VPATCH   4
 #define NSS_VBUILD   0
 #define NSS_BETA     PR_FALSE
 
 #ifndef RC_INVOKED
 
 #include "seccomon.h"
 
 typedef struct NSSInitParametersStr NSSInitParameters;
--- a/security/nss/lib/softoken/softkver.h
+++ b/security/nss/lib/softoken/softkver.h
@@ -20,16 +20,16 @@
 
 /*
  * Softoken's major version, minor version, patch level, build number,
  * and whether this is a beta release.
  *
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>[.<build number>]][ <ECC>][ <Beta>]"
  */
-#define SOFTOKEN_VERSION  "3.19.3" SOFTOKEN_ECC_STRING
+#define SOFTOKEN_VERSION  "3.19.4" SOFTOKEN_ECC_STRING
 #define SOFTOKEN_VMAJOR   3
 #define SOFTOKEN_VMINOR   19
-#define SOFTOKEN_VPATCH   3
+#define SOFTOKEN_VPATCH   4
 #define SOFTOKEN_VBUILD   0
 #define SOFTOKEN_BETA     PR_FALSE
 
 #endif /* _SOFTKVER_H_ */
--- a/security/nss/lib/util/nssutil.h
+++ b/security/nss/lib/util/nssutil.h
@@ -14,20 +14,20 @@
 
 /*
  * NSS utilities's major version, minor version, patch level, build number,
  * and whether this is a beta release.
  *
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>[.<build number>]][ <Beta>]"
  */
-#define NSSUTIL_VERSION  "3.19.3"
+#define NSSUTIL_VERSION  "3.19.4"
 #define NSSUTIL_VMAJOR   3
 #define NSSUTIL_VMINOR   19
-#define NSSUTIL_VPATCH   3
+#define NSSUTIL_VPATCH   4
 #define NSSUTIL_VBUILD   0
 #define NSSUTIL_BETA     PR_FALSE
 
 SEC_BEGIN_PROTOS
 
 /*
  * Returns a const string of the UTIL library version.
  */
--- a/security/nss/lib/util/secasn1d.c
+++ b/security/nss/lib/util/secasn1d.c
@@ -946,31 +946,116 @@ sec_asn1d_parse_more_length (sec_asn1d_s
     }
 
     if (state->pending == 0)
 	state->place = afterLength;
 
     return count;
 }
 
+/*
+ * Helper function for sec_asn1d_prepare_for_contents.
+ * Checks that a value representing a number of bytes consumed can be
+ * subtracted from a remaining length. If so, returns PR_TRUE.
+ * Otherwise, sets the error SEC_ERROR_BAD_DER, indicates that there was a
+ * decoding error in the given SEC_ASN1DecoderContext, and returns PR_FALSE.
+ */
+static PRBool
+sec_asn1d_check_and_subtract_length (unsigned long *remaining,
+                                     unsigned long consumed,
+                                     SEC_ASN1DecoderContext *cx)
+{
+    PORT_Assert(remaining);
+    PORT_Assert(cx);
+    if (!remaining || !cx) {
+        PORT_SetError (SEC_ERROR_INVALID_ARGS);
+        cx->status = decodeError;
+        return PR_FALSE;
+    }
+    if (*remaining < consumed) {
+        PORT_SetError (SEC_ERROR_BAD_DER);
+        cx->status = decodeError;
+        return PR_FALSE;
+    }
+    *remaining -= consumed;
+    return PR_TRUE;
+}
 
 static void
 sec_asn1d_prepare_for_contents (sec_asn1d_state *state)
 {
     SECItem *item;
     PLArenaPool *poolp;
     unsigned long alloc_len;
+    sec_asn1d_state *parent;
 
 #ifdef DEBUG_ASN1D_STATES
     {
         printf("Found Length %d %s\n", state->contents_length,
                state->indefinite ? "indefinite" : "");
     }
 #endif
 
+    /**
+     * The maximum length for a child element should be constrained to the
+     * length remaining in the first definite length element in the ancestor
+     * stack. If there is no definite length element in the ancestor stack,
+     * there's nothing to constrain the length of the child, so there's no
+     * further processing necessary.
+     *
+     * It's necessary to walk the ancestor stack, because it's possible to have
+     * definite length children that are part of an indefinite length element,
+     * which is itself part of an indefinite length element, and which is
+     * ultimately part of a definite length element. A simple example of this
+     * would be the handling of constructed OCTET STRINGs in BER encoding.
+     *
+     * This algorithm finds the first definite length element in the ancestor
+     * stack, if any, and if so, ensures that the length of the child element
+     * is consistent with the number of bytes remaining in the constraining
+     * ancestor element (that is, after accounting for any other sibling
+     * elements that may have been read).
+     *
+     * It's slightly complicated by the need to account both for integer
+     * underflow and overflow, as well as ensure that for indefinite length
+     * encodings, there's also enough space for the End-of-Contents (EOC)
+     * octets (Tag = 0x00, Length = 0x00, or two bytes).
+     */
+
+    /* Determine the maximum length available for this element by finding the
+     * first definite length ancestor, if any. */
+    parent = sec_asn1d_get_enclosing_construct(state);
+    while (parent && parent->indefinite) {
+        parent = sec_asn1d_get_enclosing_construct(parent);
+    }
+    /* If parent is null, state is either the outermost state / at the top of
+     * the stack, or the outermost state uses indefinite length encoding. In
+     * these cases, there's nothing external to constrain this element, so
+     * there's nothing to check. */
+    if (parent) {
+        unsigned long remaining = parent->pending;
+        parent = state;
+        do {
+            if (!sec_asn1d_check_and_subtract_length(
+                     &remaining, parent->consumed, state->top) ||
+                /* If parent->indefinite is true, parent->contents_length is
+                 * zero and this is a no-op. */
+                !sec_asn1d_check_and_subtract_length(
+                     &remaining, parent->contents_length, state->top) ||
+                /* If parent->indefinite is true, then ensure there is enough
+                 * space for an EOC tag of 2 bytes. */
+                (parent->indefinite && !sec_asn1d_check_and_subtract_length(
+                                            &remaining, 2, state->top))) {
+                /* This element is larger than its enclosing element, which is
+                 * invalid. */
+                return;
+            }
+        } while ((parent = sec_asn1d_get_enclosing_construct(parent)) &&
+                 parent->indefinite);
+    }
+
     /*
      * XXX I cannot decide if this allocation should exclude the case
      *     where state->endofcontents is true -- figure it out!
      */
     if (state->allocate) {
 	void *dest;
 
 	PORT_Assert (state->dest == NULL);
@@ -1002,31 +1087,16 @@ sec_asn1d_prepare_for_contents (sec_asn1
     }
 
     /*
      * Remember, length may be indefinite here!  In that case,
      * both contents_length and pending will be zero.
      */
     state->pending = state->contents_length;
 
-    /* If this item has definite length encoding, and 
-    ** is enclosed by a definite length constructed type,
-    ** make sure it isn't longer than the remaining space in that 
-    ** constructed type.  
-    */
-    if (state->contents_length > 0) {
-	sec_asn1d_state *parent = sec_asn1d_get_enclosing_construct(state);
-	if (parent && !parent->indefinite && 
-	    state->consumed + state->contents_length > parent->pending) {
-	    PORT_SetError (SEC_ERROR_BAD_DER);
-	    state->top->status = decodeError;
-	    return;
-	}
-    }
-
     /*
      * An EXPLICIT is nothing but an outer header, which we have
      * already parsed and accepted.  Now we need to do the inner
      * header and its contents.
      */
     if (state->explicit) {
 	state->place = afterExplicit;
 	state = sec_asn1d_push_state (state->top,
@@ -1715,20 +1785,117 @@ sec_asn1d_next_substring (sec_asn1d_stat
 	    state->top->status = decodeError;
 	    return;
 	}
 
 	state->pending -= child_consumed;
 	if (state->pending == 0)
 	    done = PR_TRUE;
     } else {
+	PRBool preallocatedString;
+	sec_asn1d_state *temp_state;
 	PORT_Assert (state->indefinite);
 
 	item = (SECItem *)(child->dest);
-	if (item != NULL && item->data != NULL) {
+
+	/**
+	 * At this point, there's three states at play:
+	 *   child: The element that was just parsed
+	 *   state: The currently processed element
+	 *   'parent' (aka state->parent): The enclosing construct
+	 *      of state, or NULL if this is the top-most element.
+	 *
+	 * This state handles both substrings of a constructed string AND
+	 * child elements of items whose template type was that of
+	 * SEC_ASN1_ANY, SEC_ASN1_SAVE, SEC_ASN1_ANY_CONTENTS, SEC_ASN1_SKIP
+	 * template, as described in sec_asn1d_prepare_for_contents. For
+	 * brevity, these will be referred to as 'string' and 'any' types.
+	 *
+	 * This leads to the following possibilities:
+	 *   1: This element is an indefinite length string, part of a
+	 *      definite length string.
+	 *   2: This element is an indefinite length string, part of an
+	 *      indefinite length string.
+	 *   3: This element is an indefinite length any, part of a
+	 *      definite length any.
+	 *   4: This element is an indefinite length any, part of an
+	 *      indefinite length any.
+	 *   5: This element is an indefinite length any and does not
+	 *      meet any of the above criteria. Note that this would include
+	 *      an indefinite length string type matching an indefinite
+	 *      length any template.
+	 *
+	 * In Cases #1 and #3, the definite length 'parent' element will
+	 * have allocated state->dest based on the parent elements definite
+	 * size. During the processing of 'child', sec_asn1d_parse_leaf will
+	 * have copied the (string, any) data directly into the offset of
+	 * dest, as appropriate, so there's no need for this class to still
+	 * store the child - it's already been processed.
+	 *
+	 * In Cases #2 and #4, dest will be set to the parent element's dest,
+	 * but dest->data will not have been allocated yet, due to the
+	 * indefinite length encoding. In this situation, it's necessary to
+	 * hold onto child (and all other children) until the EOC, at which
+	 * point, it becomes possible to compute 'state's overall length. Once
+	 * 'state' has a computed length, this can then be fed to 'parent' (via
+	 * this state), and then 'parent' can similarly compute the length of
+	 * all of its children up to the EOC, which will ultimately transit to
+	 * sec_asn1d_concat_substrings, determine the overall size needed,
+	 * allocate, and copy the contents (of all of parent's children, which
+	 * would include 'state', just as 'state' will have copied all of its
+	 * children via sec_asn1d_concat_substrings)
+	 *
+	 * The final case, Case #5, will manifest in that item->data and
+	 * item->len will be NULL/0, respectively, since this element was
+	 * indefinite-length encoded. In that case, both the tag and length will
+	 * already exist in state's subitems, via sec_asn1d_record_any_header,
+	 * and so the contents (aka 'child') should be added to that list of
+	 * items to concatenate in sec_asn1d_concat_substrings once the EOC
+	 * is encountered.
+	 *
+	 * To distinguish #2/#4 from #1/#3, it's sufficient to walk the ancestor
+	 * tree. If the current type is a string type, then the enclosing
+	 * construct will be that same type (#1/#2). If the current type is an
+	 * any type, then the enclosing construct is either an any type (#3/#4)
+	 * or some other type (#5). Since this is BER, this nesting relationship
+	 * between 'state' and 'parent' may go through several levels of
+	 * constructed encoding, so continue walking the ancestor chain until a
+	 * clear determination can be made.
+	 *
+	 * The variable preallocatedString is used to indicate Case #1/#3,
+	 * indicating an in-place copy has already occurred, and Cases #2, #4,
+	 * and #5 all have the same behaviour of adding a new substring.
+	 */
+	preallocatedString = PR_FALSE;
+	temp_state = state;
+	while (temp_state && item == temp_state->dest && temp_state->indefinite) {
+	    sec_asn1d_state *parent = sec_asn1d_get_enclosing_construct(temp_state);
+	    if (!parent || parent->underlying_kind != temp_state->underlying_kind) {
+	        /* Case #5 - Either this is a top-level construct or it is part
+	         * of some other element (e.g. a SEQUENCE), in which case, a
+	         * new item should be allocated. */
+	        break;
+	    }
+	    if (!parent->indefinite) {
+	        /* Cases #1 / #3 - A definite length ancestor exists, for which
+	         * this is a substring that has already copied into dest. */
+	        preallocatedString = PR_TRUE;
+	        break;
+	    }
+	    if (!parent->substring) {
+	        /* Cases #2 / #4 - If the parent is not a substring, but is
+	         * indefinite, then there's nothing further up that may have
+	         * preallocated dest, thus child will not have already
+		 * been copied in place, therefore it's necessary to save child
+		 * as a subitem. */
+	        break;
+	    }
+	    temp_state = parent;
+	}
+	if (item != NULL && item->data != NULL && !preallocatedString) {
 	    /*
 	     * Save the string away for later concatenation.
 	     */
 	    PORT_Assert (item->data != NULL);
 	    sec_asn1d_add_to_subitems (state, item->data, item->len, PR_FALSE);
 	    /*
 	     * Clear the child item for the next round.
 	     */