From 4decb074d8feed66201057fd663e257cf45c65d6 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 6 Nov 2023 11:11:14 +1300 Subject: [PATCH] librpc/ndr: Add support for LIBNDR_FLAG_STR_NO_EMBEDDED_NUL This requires that, other than termination, no NUL (\0) codepoints exist in the input string, because bytes beyon that will be lost in the output string. This in turn causes trouble for round-trip testing, so it is easiest to reject it upfront (on an opt-in basis). Signed-off-by: Andrew Bartlett Reviewed-by: Reviewed-by: Joseph Sutton --- librpc/idl/idl_types.h | 1 + librpc/ndr/libndr.h | 2 + librpc/ndr/ndr_string.c | 44 +++++ librpc/tests/test_ndr_string.c | 337 ++++++++++++++++++++++++++++++++- 4 files changed, 383 insertions(+), 1 deletion(-) diff --git a/librpc/idl/idl_types.h b/librpc/idl/idl_types.h index 2d063de0bc7..a21893f6562 100644 --- a/librpc/idl/idl_types.h +++ b/librpc/idl/idl_types.h @@ -5,6 +5,7 @@ #define STR_NOTERM LIBNDR_FLAG_STR_NOTERM #define STR_NULLTERM LIBNDR_FLAG_STR_NULLTERM #define STR_BYTESIZE LIBNDR_FLAG_STR_BYTESIZE +#define STR_NO_EMBEDDED_NUL LIBNDR_FLAG_STR_NO_EMBEDDED_NUL #define STR_CONFORMANT LIBNDR_FLAG_STR_CONFORMANT #define STR_CHARLEN LIBNDR_FLAG_STR_CHARLEN #define STR_UTF8 LIBNDR_FLAG_STR_UTF8 diff --git a/librpc/ndr/libndr.h b/librpc/ndr/libndr.h index b0596039526..3a453b5b168 100644 --- a/librpc/ndr/libndr.h +++ b/librpc/ndr/libndr.h @@ -150,6 +150,7 @@ struct ndr_print { #define LIBNDR_FLAG_STR_NULLTERM (1U<<6) #define LIBNDR_FLAG_STR_SIZE2 (1U<<7) #define LIBNDR_FLAG_STR_BYTESIZE (1U<<8) +#define LIBNDR_FLAG_STR_NO_EMBEDDED_NUL (1U<<9) #define LIBNDR_FLAG_STR_CONFORMANT (1U<<10) #define LIBNDR_FLAG_STR_CHARLEN (1U<<11) #define LIBNDR_FLAG_STR_UTF8 (1U<<12) @@ -162,6 +163,7 @@ struct ndr_print { LIBNDR_FLAG_STR_NULLTERM | \ LIBNDR_FLAG_STR_SIZE2 | \ LIBNDR_FLAG_STR_BYTESIZE | \ + LIBNDR_FLAG_STR_NO_EMBEDDED_NUL | \ LIBNDR_FLAG_STR_CONFORMANT | \ LIBNDR_FLAG_STR_CHARLEN | \ LIBNDR_FLAG_STR_UTF8 | \ diff --git a/librpc/ndr/ndr_string.c b/librpc/ndr/ndr_string.c index 988243abb0b..57a49e34c17 100644 --- a/librpc/ndr/ndr_string.c +++ b/librpc/ndr/ndr_string.c @@ -50,6 +50,12 @@ _PUBLIC_ enum ndr_err_code ndr_pull_string(struct ndr_pull *ndr, ndr_flags_type flags &= ~LIBNDR_FLAG_STR_ASCII; } + /* + * We will check this flag, but from the unmodified + * ndr->flags, so just remove it from flags + */ + flags &= ~LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + if (flags & LIBNDR_FLAG_STR_UTF8) { chset = CH_UTF8; byte_mul = 1; @@ -187,7 +193,39 @@ _PUBLIC_ enum ndr_err_code ndr_pull_string(struct ndr_pull *ndr, ndr_flags_type if (converted_size > 0 && as[converted_size-1] == '\0') { DEBUG(6,("short string '%s', sent with NULL termination despite NOTERM flag in IDL\n", as)); } + /* + * We check the original ndr->flags as it has already + * been removed from the local variable flags + */ + if (ndr->flags & LIBNDR_FLAG_STR_NO_EMBEDDED_NUL) { + size_t strlen_of_unix_string = strlen(as); + if (strlen_of_unix_string != converted_size) { + return ndr_pull_error(ndr, NDR_ERR_CHARCNV, + "Embedded NUL at position %zu in " + "converted string " + "(and therefore source string) " + "despite " + "LIBNDR_FLAG_NO_EMBEDDED_NUL\n", + strlen_of_unix_string); + } + } } else { + /* + * We check the original ndr->flags as it has already + * been removed from the local variable flags + */ + if (ndr->flags & LIBNDR_FLAG_STR_NO_EMBEDDED_NUL) { + size_t strlen_of_unix_string = strlen(as); + if (converted_size > 0 && strlen_of_unix_string != converted_size - 1) { + return ndr_pull_error(ndr, NDR_ERR_CHARCNV, + "Embedded NUL at position %zu in " + "converted string " + "(and therefore source string) " + "despite " + "LIBNDR_FLAG_NO_EMBEDDED_NUL\n", + strlen_of_unix_string); + } + } if (converted_size > 0 && as[converted_size-1] != '\0') { DEBUG(6,("long string '%s', send without NULL termination (which was expected)\n", as)); } @@ -228,6 +266,12 @@ _PUBLIC_ enum ndr_err_code ndr_push_string(struct ndr_push *ndr, ndr_flags_type flags &= ~LIBNDR_FLAG_STR_ASCII; } + /* + * We will check this flag, but from the unmodified + * ndr->flags, so just remove it from flags + */ + flags &= ~LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + if (flags & LIBNDR_FLAG_STR_UTF8) { chset = CH_UTF8; byte_mul = 1; diff --git a/librpc/tests/test_ndr_string.c b/librpc/tests/test_ndr_string.c index 7c8ded4df42..3250f394270 100644 --- a/librpc/tests/test_ndr_string.c +++ b/librpc/tests/test_ndr_string.c @@ -192,6 +192,325 @@ static void test_pull_string_array(void **state) TALLOC_FREE(mem_ctx); } +static void test_pull_string_zero_len_utf8_NOTERM_STR_NO_EMBEDDED_NUL(void **state) +{ + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x0, 0x0 }; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NOTERM | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_SUCCESS); + assert_non_null(s); + assert_string_equal(s, ""); + assert_int_equal(sizeof(data), ndr.offset); + +} + +static void test_pull_string_utf8_nul_term_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x2, 0x0, 'a', 0x0 }; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_SUCCESS); + assert_non_null(s); + assert_string_equal(s, "a"); + assert_int_equal(sizeof(data), ndr.offset); + +} + +static void test_pull_string_utf8_nul_term_NOTERM_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x2, 0x0, 'a', 0x0 }; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NOTERM | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_CHARCNV); + assert_int_equal(2, ndr.offset); + +} + +static void test_pull_string_utf8_nullterm_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x4, 0x0, 'a', 'b', 'c', 0x0}; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_SUCCESS); + assert_non_null(s); + assert_string_equal(s, "abc"); + assert_int_equal(sizeof(data), ndr.offset); + +} + +static void test_pull_string_utf8_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x3, 0x0, 'a', 'b', 'c'}; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_CHARCNV); + assert_int_equal(2, ndr.offset); + +} + +static void test_pull_string_utf8_NOTERM_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x3, 0x0, 'a', 'b', 'c'}; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NOTERM | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_SUCCESS); + assert_non_null(s); + assert_string_equal(s, "abc"); + assert_int_equal(sizeof(data), ndr.offset); + +} + +static void test_pull_string_utf8_nullterm_NOTERM_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x4, 0x0, 'a', 'b', 'c', 0x0}; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NOTERM | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_CHARCNV); + assert_int_equal(2, ndr.offset); + +} + +static void test_pull_string_utf8_LIBNDR_FLAG_STR_NOTERM_STR_NO_EMBEDDED_NUL_fail(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x3, 0x0, 'a', 0x0, 'a'}; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NOTERM | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_CHARCNV); + assert_int_equal(2, ndr.offset); + +} + +static void test_pull_string_utf16_LIBNDR_FLAG_STR_NOTERM_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x3, 0x0, 'a', 0x0, 'b', 0x0, 'c', 0x0}; + + ndr.flags = LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NOTERM | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_SUCCESS); + assert_non_null(s); + assert_string_equal(s, "abc"); + assert_int_equal(sizeof(data), ndr.offset); + +} + +static void test_pull_string_utf16_LIBNDR_FLAG_STR_NOTERM_STR_NO_EMBEDDED_NUL_fail(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x3, 0x0, 'a', 0x0, 0x0, 0x0, 'c', 0x0}; + + ndr.flags = LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NOTERM | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_CHARCNV); + assert_int_equal(2, ndr.offset); + +} + +static void test_pull_string_zero_len_utf8_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x0, 0x0 }; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_SUCCESS); + assert_non_null(s); + assert_string_equal(s, ""); + assert_int_equal(sizeof(data), ndr.offset); + +} + +static void test_pull_string_nul_only_utf8_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = { 0x2, 0x0, 0x0, 0x0 }; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_SIZE2 | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_CHARCNV); + assert_int_equal(2, ndr.offset); + +} + +static void test_pull_string_nul_term_utf8_NOTERM_NDR_REMAINING_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = {'a', 'b', 'c', 0x0 }; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_NOTERM | LIBNDR_FLAG_REMAINING | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_CHARCNV); + assert_int_equal(0, ndr.offset); + +} + +static void test_pull_string_utf8_NOTERM_NDR_REMAINING_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = {'a', 'b', 'c' }; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_NOTERM | LIBNDR_FLAG_REMAINING | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_SUCCESS); + assert_non_null(s); + assert_string_equal(s, "abc"); + assert_int_equal(sizeof(data), ndr.offset); + +} + +static void test_pull_string_nul_term_utf8_STR_NULLTERM_NDR_REMAINING_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = {'a', 'b', 'c', 0x0 }; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_NULLTERM | LIBNDR_FLAG_REMAINING | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_SUCCESS); + assert_non_null(s); + assert_string_equal(s, "abc"); + assert_int_equal(sizeof(data), ndr.offset); + +} + +static void test_pull_string_utf8_NDR_REMAINING_STR_NULLTERM_STR_NO_EMBEDDED_NUL(void **state) +{ + + struct ndr_pull ndr = {0}; + enum ndr_err_code err; + ndr_flags_type flags = NDR_SCALARS; + const char *s = NULL; + uint8_t data[] = {'a', 'b', 'c' }; + + ndr.flags = LIBNDR_FLAG_STR_UTF8 | LIBNDR_FLAG_STR_NULLTERM | LIBNDR_FLAG_REMAINING | LIBNDR_FLAG_STR_NO_EMBEDDED_NUL; + + ndr.data = data; + ndr.data_size = sizeof(data); + err = ndr_pull_string(&ndr, flags, &s); + assert_int_equal(err, NDR_ERR_CHARCNV); + assert_int_equal(0, ndr.offset); + +} + int main(int argc, const char **argv) { const struct CMUnitTest tests[] = { @@ -199,7 +518,23 @@ int main(int argc, const char **argv) cmocka_unit_test(test_pull_string_len_1_nul_term), cmocka_unit_test(test_pull_string_len_2_nul_term), cmocka_unit_test(test_ndr_string_n_length), - cmocka_unit_test(test_pull_string_array) + cmocka_unit_test(test_pull_string_array), + cmocka_unit_test(test_pull_string_zero_len_utf8_NOTERM_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf8_nul_term_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf8_nul_term_NOTERM_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf8_nullterm_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf8_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf8_NOTERM_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf8_nullterm_NOTERM_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf8_LIBNDR_FLAG_STR_NOTERM_STR_NO_EMBEDDED_NUL_fail), + cmocka_unit_test(test_pull_string_utf16_LIBNDR_FLAG_STR_NOTERM_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf16_LIBNDR_FLAG_STR_NOTERM_STR_NO_EMBEDDED_NUL_fail), + cmocka_unit_test(test_pull_string_zero_len_utf8_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_nul_only_utf8_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_nul_term_utf8_NOTERM_NDR_REMAINING_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf8_NOTERM_NDR_REMAINING_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_nul_term_utf8_STR_NULLTERM_NDR_REMAINING_STR_NO_EMBEDDED_NUL), + cmocka_unit_test(test_pull_string_utf8_NDR_REMAINING_STR_NULLTERM_STR_NO_EMBEDDED_NUL) }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.47.3