Add more punycode tests and remove ossl_a2ucompare()

The unused and untested internal function ossl_a2ucompare() has been
removed.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/20177)

diff --git a/crypto/punycode.c b/crypto/punycode.c
index ad88495f6c50647181445165e1a0d240a83d8616..332817763dc6e24faa1b947bdb89b76da9a6298f 100644 (file)
--- a/crypto/punycode.c
+++ b/crypto/punycode.c
@@ -310,22 +310,3 @@ int ossl_a2ulabel(const char *in, char *out, size_t outlen)
     WPACKET_cleanup(&pkt);
     return result;
 }
-
-/*-
- * a MUST be A-label
- * u MUST be U-label
- * Returns 0 if compared values are equal
- * 1 if not
- * -1 in case of errors
- */
-
-int ossl_a2ucompare(const char *a, const char *u)
-{
-    char a_ulabel[LABEL_BUF_SIZE + 1];
-    size_t a_size = sizeof(a_ulabel);
-
-    if (ossl_a2ulabel(a, a_ulabel, a_size) <= 0)
-        return -1;
-
-    return strcmp(a_ulabel, u) != 0;
-}

diff --git a/doc/internal/man3/ossl_punycode_decode.pod b/doc/internal/man3/ossl_punycode_decode.pod
index 8c9484889b2e0becc25f76b37928dfc444c68b27..7a20864bcd8c00910d304ee0f58d20bf347cd32a 100644 (file)
--- a/doc/internal/man3/ossl_punycode_decode.pod
+++ b/doc/internal/man3/ossl_punycode_decode.pod
@@ -2,7 +2,7 @@
 
 =head1 NAME
 
-ossl_punycode_decode, ossl_a2ulabel, ossl_a2ucompare
+ossl_punycode_decode, ossl_a2ulabel
 - internal punycode-related functions
 
 =head1 SYNOPSIS
@@ -14,8 +14,6 @@ ossl_punycode_decode, ossl_a2ulabel, ossl_a2ucompare
 
   int ossl_a2ulabel(const char *in, char *out, size_t outlen);
 
-  int ossl_a2ucompare(const char *a, const char *u);
-
 =head1 DESCRIPTION
 
 PUNYCODE encoding introduced in RFCs 3490-3492 is widely used for
@@ -25,9 +23,6 @@ such as RFC 8398, require comparison of hostnames encoded in UTF-8 charset.
 ossl_a2ulabel() decodes NUL-terminated hostname from PUNYCODE to UTF-8,
 using a provided buffer for output.  The output buffer is NUL-terminated.
 
-ossl_a2ucompare() accepts two NUL-terminated hostnames, decodes the 1st
-from PUNYCODE to UTF-8 and compares it with the 2nd one as is.
-
 ossl_punycode_decode() decodes one label (one dot-separated part) from
 a hostname, with stripped PUNYCODE marker I<xn-->.
 
@@ -36,9 +31,6 @@ a hostname, with stripped PUNYCODE marker I<xn-->.
 ossl_a2ulabel() returns 1 on success, 0 if the output buffer is too small and
 -1 if an invalid PUNYCODE string is passed or another error occurs.
 
-ossl_a2ucompare() returns 1 on non-equal strings, 0 on equal strings,
--1 when an invalid PUNYCODE string is passed or another error occurs.
-
 ossl_punycode_decode() returns 1 on success, 0 on error. On success,
 *pout_length contains the number of codepoints decoded.
 

diff --git a/include/crypto/punycode.h b/include/crypto/punycode.h
index e448dadbbd8049891df7ae544229dc1909605c44..554819a2809e09267908c9b896fed55bde06b802 100644 (file)
--- a/include/crypto/punycode.h
+++ b/include/crypto/punycode.h
@@ -22,6 +22,4 @@ int ossl_punycode_decode (
 
 int ossl_a2ulabel(const char *in, char *out, size_t outlen);
 
-int ossl_a2ucompare(const char *a, const char *u);
-
 #endif

diff --git a/test/punycode_test.c b/test/punycode_test.c
index 5b8b0bd2720247006c4616e11027889e931bbab2..8a4ea0dc41717bd37c1e6ada81042d4284a7b7a4 100644 (file)
--- a/test/punycode_test.c
+++ b/test/punycode_test.c
@@ -20,6 +20,11 @@ static const struct puny_test {
     unsigned int raw[50];
     const char *encoded;
 } puny_cases[] = {
+    { /* Test of 4 byte codepoint using smileyface emoji */
+        { 0x1F600
+        },
+        "e28h"
+    },
     /* Test cases from RFC 3492 */
     {   /* Arabic (Egyptian) */
         { 0x0644, 0x064A, 0x0647, 0x0645, 0x0627, 0x0628, 0x062A, 0x0643, 0x0644,
@@ -164,9 +169,28 @@ static int test_punycode(int n)
     return 1;
 }
 
+static const struct bad_decode_test {
+    size_t outlen;
+    const char input[20];
+} bad_decode_tests[] = {
+    { 20, "xn--e-*" },   /* bad digit '*' */
+    { 10, "xn--e-999" }, /* loop > enc_len */
+    { 20, "xn--e-999999999" }, /* Too big */
+    { 20, {'x', 'n', '-', '-', (char)0x80, '-' } }, /* Not basic */
+    { 20, "xn--e-Oy65t" }, /* codepoint > 0x10FFFF */
+};
+
+static int test_a2ulabel_bad_decode(int tst)
+{
+    char out[20];
+
+    return TEST_int_eq(ossl_a2ulabel(bad_decode_tests[tst].input, out, bad_decode_tests[tst].outlen), -1);
+}
+
 static int test_a2ulabel(void)
 {
     char out[50];
+    char in[530] = { 0 };
 
     /*
      * The punycode being passed in and parsed is malformed but we're not
@@ -180,6 +204,18 @@ static int test_a2ulabel(void)
             || !TEST_int_eq(ossl_a2ulabel("xn--a.b.c", out, 7), 1)
             || !TEST_str_eq(out,"\xc2\x80.b.c"))
         return 0;
+
+    /* Test 4 byte smiley face */
+    if (!TEST_int_eq(ossl_a2ulabel("xn--e28h.com", out, 10), 1))
+        return 0;
+
+    /* Test that we dont overflow the fixed internal buffer of 512 bytes when the starting bytes are copied */
+    strcpy(in, "xn--");
+    memset(in + 4, 'e', 513);
+    memcpy(in + 517, "-3ya", 4);
+    if (!TEST_int_eq(ossl_a2ulabel(in, out, 50), -1))
+        return 0;
+
     return 1;
 }
 
@@ -253,5 +289,6 @@ int setup_tests(void)
     ADD_TEST(test_dotted_overflow);
     ADD_TEST(test_a2ulabel);
     ADD_TEST(test_puny_overrun);
+    ADD_ALL_TESTS(test_a2ulabel_bad_decode, OSSL_NELEM(bad_decode_tests));
     return 1;
 }