Bug 1646520 - Stricter leading-zero checks for ASN.1 INTEGER values. r=jcjNSS_3_54_BETA1

This patch adjusts QuickDER to strictly enforce INTEGER encoding with respect to leading zeros:
- If the MSB of the first (value) octet is set, a single zero byte MAY be present to make the value positive. This singular pad byte is removed.
- Otherwise, the first octet must not be zero.

Differential Revision: https://phabricator.services.mozilla.com/D80221

1 files changed, 7 insertions, 4 deletions

diff --git a/lib/util/quickder.c b/lib/util/quickder.c
index 70ae42b27..d694581e1 100644
--- a/lib/util/quickder.c
+++ b/lib/util/quickder.c
@@ -742,15 +742,18 @@ DecodeItem(void* dest,
                     switch (tagnum) {
                         /* special cases of primitive types */
                         case SEC_ASN1_INTEGER: {
-                            /* remove leading zeroes if the caller requested
-                               siUnsignedInteger
-                               This is to allow RSA key operations to work */
                             SECItem* destItem = (SECItem*)((char*)dest +
                                                            templateEntry->offset);
                             if (destItem && (siUnsignedInteger == destItem->type)) {
-                                while (temp.len > 1 && temp.data[0] == 0) { /* leading 0 */
+                                /* A leading 0 is only allowed when a value
+                                 * would otherwise be interpreted as negative. */
+                                if (temp.len > 1 && temp.data[0] == 0) {
                                     temp.data++;
                                     temp.len--;
+                                    if (!(temp.data[0] & 0x80)) {
+                                        PORT_SetError(SEC_ERROR_BAD_DER);
+                                        rv = SECFailure;
+                                    }
                                 }
                             }
                             break;