NUL is not printable.

strchr is interprets the trailing NUL as part of the string, so
is_printable thought NUL was allowed. Just write the code in the obvious
way and let the compiler figure it out. (It seems to make a clever
bitmask or something.)

Update-Note: ASN1_mbstring_ncopy will no longer allow PrintableString
for strings containing NUL.

Change-Id: I3675191ceb44c06f0ac7b430f88272cabf392d35
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49065
Reviewed-by: Adam Langley <agl@google.com>

crypto/asn1/a_mbstr.c

@@ -288,21 +288,14 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len,
 /* Return 1 if the character is permitted in a PrintableString */
 static int is_printable(uint32_t value)
 {
-    int ch;
-    if (value > 0x7f)
+    if (value > 0x7f) {
         return 0;
-    ch = (int)value;
-    /*
-     * Note: we can't use 'isalnum' because certain accented characters may
-     * count as alphanumeric in some environments.
-     */
-    if ((ch >= 'a') && (ch <= 'z'))
-        return 1;
-    if ((ch >= 'A') && (ch <= 'Z'))
-        return 1;
-    if ((ch >= '0') && (ch <= '9'))
-        return 1;
-    if ((ch == ' ') || strchr("'()+,-./:=?", ch))
-        return 1;
-    return 0;
+    }
+    /* Note we cannot use |isalnum| because it is locale-dependent. */
+    return ('a' <= value && value <= 'z') ||  //
+           ('A' <= value && value <= 'Z') ||  //
+           ('0' <= value && value <= '9') ||  //
+           value == ' ' || value == '\'' || value == '(' || value == ')' ||
+           value == '+' || value == ',' || value == '-' || value == '.' ||
+           value == '/' || value == ':' || value == '=' || value == '?';
 }

crypto/asn1/asn1_test.cc

@@ -833,6 +833,13 @@ TEST(ASN1Test, MBString) {
       // Given a choice of formats, we pick the smallest that fits.
       {MBSTRING_UTF8, {}, kAll, V_ASN1_PRINTABLESTRING, {}, 0},
       {MBSTRING_UTF8, {'a'}, kAll, V_ASN1_PRINTABLESTRING, {'a'}, 1},
+      {MBSTRING_UTF8,
+       {'a', 'A', '0', '\'', '(', ')', '+', ',', '-', '.', '/', ':', '=', '?'},
+       kAll,
+       V_ASN1_PRINTABLESTRING,
+       {'a', 'A', '0', '\'', '(', ')', '+', ',', '-', '.', '/', ':', '=', '?'},
+       14},
+      {MBSTRING_UTF8, {'*'}, kAll, V_ASN1_IA5STRING, {'*'}, 1},
       {MBSTRING_UTF8, {'\n'}, kAll, V_ASN1_IA5STRING, {'\n'}, 1},
       {MBSTRING_UTF8,
        {0xc2, 0x80 /* U+0080 */},
@@ -859,6 +866,10 @@ TEST(ASN1Test, MBString) {
        {0xf0, 0x90, 0x80, 0x80},
        1},
 
+      // NUL is not printable. It should also not terminate iteration.
+      {MBSTRING_UTF8, {0}, kAll, V_ASN1_IA5STRING, {0}, 1},
+      {MBSTRING_UTF8, {0, 'a'}, kAll, V_ASN1_IA5STRING, {0, 'a'}, 2},
+
       // When a particular format is specified, we use it.
       {MBSTRING_UTF8,
        {'a'},