NSS: Don't use printf(3) on user provided strings.

This also fixes several corner cases and crashers.

It's not prudent to pass user input to (even admin) input as a
format string to printf, and various distros now check for this.
This can cause accessing memory incorrectly, and various also
various libc abort()'s.

In addition various assumptions were made about full_name_format
that aren't necessarily the case if the user uses a more complex
format.

Use safe-printf.c implementation for formatting full_name_format.

Adapt the NSS resolver so it doesn't barf on formatted strings that
are shorter than expected given a full_name_format.

Tests added and updated appropriately.

3 files changed, 12 insertions, 36 deletions

diff --git a/src/tests/cmocka/test_fqnames.c b/src/tests/cmocka/test_fqnames.c
index e0735083a..90f60c5be 100644
--- a/src/tests/cmocka/test_fqnames.c
+++ b/src/tests/cmocka/test_fqnames.c
@@ -76,7 +76,6 @@ void test_default(void **state)
     char *fqdn;
     const int fqdn_size = 255;
     char fqdn_s[fqdn_size];
-    size_t domsize;
 
     if (test_ctx == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Type mismatch\n"));
@@ -87,7 +86,6 @@ void test_default(void **state)
                                    "(?P<name>[^@]+)@?(?P<domain>[^@]*$)",
                                    "%1$s@%2$s", &test_ctx->nctx);
     assert_int_equal(ret, EOK);
-    assert_int_equal(test_ctx->nctx->fq_flags, FQ_FMT_NAME | FQ_FMT_DOMAIN);
 
     fqdn = sss_tc_fqname(test_ctx, test_ctx->nctx, test_ctx->dom, NAME);
     assert_non_null(fqdn);
@@ -98,9 +96,6 @@ void test_default(void **state)
     assert_int_equal(ret + 1, sizeof(NAME"@"DOMNAME));
     assert_string_equal(fqdn_s, NAME"@"DOMNAME);
 
-    domsize = sss_fqdom_len(test_ctx->nctx, test_ctx->dom);
-    assert_int_equal(domsize, sizeof(DOMNAME)-1);
-
     talloc_free(test_ctx->nctx);
 }
 
@@ -113,7 +108,6 @@ void test_all(void **state)
     char *fqdn;
     const int fqdn_size = 255;
     char fqdn_s[fqdn_size];
-    size_t domsize;
 
     if (test_ctx == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Type mismatch\n"));
@@ -124,8 +118,6 @@ void test_all(void **state)
                                    "(?P<name>[^@]+)@?(?P<domain>[^@]*$)",
                                    "%1$s@%2$s@%3$s", &test_ctx->nctx);
     assert_int_equal(ret, EOK);
-    assert_int_equal(test_ctx->nctx->fq_flags,
-                     FQ_FMT_NAME | FQ_FMT_DOMAIN | FQ_FMT_FLAT_NAME);
 
     fqdn = sss_tc_fqname(test_ctx, test_ctx->nctx, test_ctx->dom, NAME);
     assert_non_null(fqdn);
@@ -136,9 +128,6 @@ void test_all(void **state)
     assert_int_equal(ret + 1, sizeof(NAME"@"DOMNAME"@"FLATNAME));
     assert_string_equal(fqdn_s, NAME"@"DOMNAME"@"FLATNAME);
 
-    domsize = sss_fqdom_len(test_ctx->nctx, test_ctx->dom);
-    assert_int_equal(domsize, sizeof(DOMNAME)-1 + sizeof(FLATNAME)-1);
-
     talloc_free(test_ctx->nctx);
 }
 
@@ -151,7 +140,6 @@ void test_flat(void **state)
     char *fqdn;
     const int fqdn_size = 255;
     char fqdn_s[fqdn_size];
-    size_t domsize;
 
     if (test_ctx == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Type mismatch\n"));
@@ -162,7 +150,6 @@ void test_flat(void **state)
                                    "(?P<name>[^@]+)@?(?P<domain>[^@]*$)",
                                    "%1$s@%3$s", &test_ctx->nctx);
     assert_int_equal(ret, EOK);
-    assert_int_equal(test_ctx->nctx->fq_flags, FQ_FMT_NAME | FQ_FMT_FLAT_NAME);
 
     fqdn = sss_tc_fqname(test_ctx, test_ctx->nctx, test_ctx->dom, NAME);
     assert_non_null(fqdn);
@@ -173,9 +160,6 @@ void test_flat(void **state)
     assert_int_equal(ret + 1, sizeof(NAME"@"FLATNAME));
     assert_string_equal(fqdn_s, NAME"@"FLATNAME);
 
-    domsize = sss_fqdom_len(test_ctx->nctx, test_ctx->dom);
-    assert_int_equal(domsize, sizeof(FLATNAME)-1);
-
     talloc_free(test_ctx->nctx);
 }
 
@@ -188,7 +172,6 @@ void test_flat_fallback(void **state)
     char *fqdn;
     const int fqdn_size = 255;
     char fqdn_s[fqdn_size];
-    size_t domsize;
 
     if (test_ctx == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Type mismatch\n"));
@@ -199,7 +182,6 @@ void test_flat_fallback(void **state)
                                    "(?P<name>[^@]+)@?(?P<domain>[^@]*$)",
                                    "%1$s@%3$s", &test_ctx->nctx);
     assert_int_equal(ret, EOK);
-    assert_int_equal(test_ctx->nctx->fq_flags, FQ_FMT_NAME | FQ_FMT_FLAT_NAME);
 
     test_ctx->dom->flat_name = NULL;
 
@@ -215,9 +197,6 @@ void test_flat_fallback(void **state)
     assert_int_equal(ret + 1, sizeof(NAME"@"DOMNAME));
     assert_string_equal(fqdn_s, NAME"@"DOMNAME);
 
-    domsize = sss_fqdom_len(test_ctx->nctx, test_ctx->dom);
-    assert_int_equal(domsize, sizeof(DOMNAME)-1);
-
     talloc_free(test_ctx->nctx);
 }
 
diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index d1a4d4bd8..4db108a79 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -540,8 +540,8 @@ void test_nss_getpwnam_fqdn(void **state)
  * Check that FQDN processing is able to handle arbitrarily sized
  * delimeter
  */
-static int test_nss_getpwnam_check_resize_fqdn(uint32_t status,
-                                               uint8_t *body, size_t blen)
+static int test_nss_getpwnam_check_fancy_fqdn(uint32_t status,
+                                              uint8_t *body, size_t blen)
 {
     struct passwd pwd;
     errno_t ret;
@@ -555,29 +555,28 @@ static int test_nss_getpwnam_check_resize_fqdn(uint32_t status,
 
     assert_int_equal(pwd.pw_uid, 125);
     assert_int_equal(pwd.pw_gid, 458);
-    assert_string_equal(pwd.pw_name, "testuser_fqdn_resize@@@@@"TEST_DOM_NAME);
+    assert_string_equal(pwd.pw_name, "testuser_fqdn_fancy@@@@@"TEST_DOM_NAME);
     assert_string_equal(pwd.pw_shell, "/bin/sh");
     return EOK;
 }
 
-void test_nss_getpwnam_fqdn_resize(void **state)
+void test_nss_getpwnam_fqdn_fancy(void **state)
 {
     errno_t ret;
 
     /* Prime the cache with a valid user */
     ret = sysdb_add_user(nss_test_ctx->tctx->dom,
-                         "testuser_fqdn_resize", 125, 458, "test user",
+                         "testuser_fqdn_fancy", 125, 458, "test user",
                          "/home/testuser", "/bin/sh", NULL,
                          NULL, 300, 0);
     assert_int_equal(ret, EOK);
 
-    mock_input_user_or_group("testuser_fqdn_resize@"TEST_DOM_NAME);
+    mock_input_user_or_group("testuser_fqdn_fancy@"TEST_DOM_NAME);
     will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWNAM);
     mock_fill_user();
-    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
 
     /* Query for that user, call a callback when command finishes */
-    set_cmd_cb(test_nss_getpwnam_check_resize_fqdn);
+    set_cmd_cb(test_nss_getpwnam_check_fancy_fqdn);
     nss_test_ctx->cctx->rctx->domains[0].fqnames = true;
     ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
                           nss_test_ctx->nss_cmds);
@@ -1298,7 +1297,7 @@ void nss_subdom_test_setup(void **state)
     nss_test_ctx->subdom = subdomain;
 }
 
-void nss_fqdn_resize_test_setup(void **state)
+void nss_fqdn_fancy_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
         { "enumerate", "false" },
@@ -1339,8 +1338,8 @@ int main(int argc, const char *argv[])
                                  nss_test_setup, nss_test_teardown),
         unit_test_setup_teardown(test_nss_getpwnam_fqdn,
                                  nss_fqdn_test_setup, nss_test_teardown),
-        unit_test_setup_teardown(test_nss_getpwnam_fqdn_resize,
-                                 nss_fqdn_resize_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getpwnam_fqdn_fancy,
+                                 nss_fqdn_fancy_test_setup, nss_test_teardown),
         unit_test_setup_teardown(test_nss_getgrnam_no_members,
                                  nss_test_setup, nss_test_teardown),
         unit_test_setup_teardown(test_nss_getgrnam_members,
diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c
index eeef9ee0c..13a67f756 100644
--- a/src/tests/cmocka/test_utils.c
+++ b/src/tests/cmocka/test_utils.c
@@ -531,8 +531,7 @@ void test_sss_names_init(void **state)
     assert_int_equal(ret, EOK);
     assert_non_null(names_ctx);
     assert_string_equal(names_ctx->re_pattern, GLOBAL_RE_EXPRESSION);
-    assert_string_equal(names_ctx->fq_fmt, GLOBAL_FULL_NAME_FORMAT"%3$s");
-    assert_int_equal(names_ctx->fq_flags, FQ_FMT_NAME|FQ_FMT_DOMAIN);
+    assert_string_equal(names_ctx->fq_fmt, GLOBAL_FULL_NAME_FORMAT);
 
     talloc_free(names_ctx);
 
@@ -541,8 +540,7 @@ void test_sss_names_init(void **state)
     assert_int_equal(ret, EOK);
     assert_non_null(names_ctx);
     assert_string_equal(names_ctx->re_pattern, DOMAIN_RE_EXPRESSION);
-    assert_string_equal(names_ctx->fq_fmt, DOMAIN_FULL_NAME_FORMAT"%2$s");
-    assert_int_equal(names_ctx->fq_flags, FQ_FMT_NAME|FQ_FMT_FLAT_NAME);
+    assert_string_equal(names_ctx->fq_fmt, DOMAIN_FULL_NAME_FORMAT);
 
     talloc_free(names_ctx);
 }