From 23e65561e28f705f8f59128470aaf89bdbdb84fa Mon Sep 17 00:00:00 2001 From: slontis Date: Tue, 31 Jan 2023 10:50:22 +1000 Subject: [PATCH] Add more punycode tests and remove ossl_a2ucompare() The unused and untested internal function ossl_a2ucompare() has been removed. Reviewed-by: Paul Dale Reviewed-by: Dmitry Belyavskiy (Merged from https://github.com/openssl/openssl/pull/20177) --- crypto/punycode.c | 19 ----------- doc/internal/man3/ossl_punycode_decode.pod | 10 +----- include/crypto/punycode.h | 2 -- test/punycode_test.c | 37 ++++++++++++++++++++++ 4 files changed, 38 insertions(+), 30 deletions(-) diff --git a/crypto/punycode.c b/crypto/punycode.c index ad88495f6c..332817763d 100644 --- 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 8c9484889b..7a20864bcd 100644 --- 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. @@ -36,9 +31,6 @@ a hostname, with stripped PUNYCODE marker I. 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 e448dadbbd..554819a280 100644 --- 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 5b8b0bd272..8a4ea0dc41 100644 --- 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; } -- 2.34.1