From 9c59d009537d0792c0f15b246064b98e97fbe278 Mon Sep 17 00:00:00 2001
From: Simon Pichugin <spichugi@redhat.com>
Date: Apr 02 2020 14:06:34 +0000
Subject: Issue 50869 - Setting nsslapd-allowed-sasl-mechanisms truncates the value


Bug Description: Adding multiple mechanisms to nsslapd-allowed-sasl-mechanisms ignores all but one of the mechanisms specified.

Fix Description: The issue happens because we use the same memory address
for 'char *' for slapdFrontendConfig->allowed_sasl_mechs and
for slapdFrontendConfig->allowed_sasl_mechs_array.
So when we split the 'char *' into the 'char **' with ' ' delimetr,
allowed_sasl_mechs has only the first element becuase ' ' is set to 0 now.

Define a separate 'char *' for the array.
Add a test for the issue.

https://pagure.io/389-ds-base/issue/50869

Reviewed by: mreynolds, firstyear, tbordaz (Thanks!!)

---

diff --git a/dirsrvtests/tests/suites/sasl/allowed_mechs_test.py b/dirsrvtests/tests/suites/sasl/allowed_mechs_test.py
index 7c807f3..99ec46f 100644
--- a/dirsrvtests/tests/suites/sasl/allowed_mechs_test.py
+++ b/dirsrvtests/tests/suites/sasl/allowed_mechs_test.py
@@ -176,6 +176,32 @@ def test_basic_feature(topology_st):
     assert(set(final_mechs) == set(orig_mechs))
 
 
+@pytest.mark.bz1816854
+@pytest.mark.ds50869
+def test_config_set_few_mechs(topology_st):
+    """Test that we can successfully set multiple values to nsslapd-allowed-sasl-mechanisms
+
+    :id: d7c3c58b-4fbe-42ab-a8d4-9dd362916d5f
+    :setup: Standalone instance
+    :steps:
+        1. Set nsslapd-allowed-sasl-mechanisms to "PLAIN GSSAPI"
+        2. Verify nsslapd-allowed-sasl-mechanisms has the values
+    :expectedresults:
+        1. Operation should be successful
+        2. Operation should be successful
+    """
+
+    standalone = topology_st.standalone
+
+    standalone.log.info("Set nsslapd-allowed-sasl-mechanisms to 'PLAIN GSSAPI'")
+    standalone.config.set('nsslapd-allowed-sasl-mechanisms', 'PLAIN GSSAPI')
+
+    standalone.log.info("Verify nsslapd-allowed-sasl-mechanisms has the values")
+    allowed_mechs = standalone.config.get_attr_val_utf8('nsslapd-allowed-sasl-mechanisms')
+    assert('PLAIN' in allowed_mechs)
+    assert('GSSAPI' in allowed_mechs)
+
+
 if __name__ == '__main__':
     # Run isolated
     # -s for DEBUG mode
diff --git a/ldap/servers/slapd/charray.c b/ldap/servers/slapd/charray.c
index 6175491..c27c5da 100644
--- a/ldap/servers/slapd/charray.c
+++ b/ldap/servers/slapd/charray.c
@@ -325,6 +325,8 @@ slapi_str2charray(char *str, char *brkstr)
 /*
  * extended version of str2charray lets you disallow
  * duplicate values into the array.
+ * Also, "char *str" should be a temporary string which is freed afterwards.
+ * the string is changed during this function execution
  */
 char **
 slapi_str2charray_ext(char *str, char *brkstr, int allow_dups)
diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c
index 2a58232..7d275eb 100644
--- a/ldap/servers/slapd/libglobs.c
+++ b/ldap/servers/slapd/libglobs.c
@@ -7282,9 +7282,12 @@ config_set_allowed_sasl_mechs(const char *attrname, char *value, char *errorbuf 
     /* During a reset, the value is "", so we have to handle this case. */
     if (strcmp(value, "") != 0) {
         char *nval = slapi_ch_strdup(value);
+        /* A separate variable is used because slapi_str2charray_ext can change it and nval'd become corrupted */
+        char *tmp_array_nval;
 
         /* cyrus sasl doesn't like comma separated lists */
         remove_commas(nval);
+        tmp_array_nval = slapi_ch_strdup(nval);
 
         if (invalid_sasl_mech(nval)) {
             slapi_log_err(SLAPI_LOG_ERR, "config_set_allowed_sasl_mechs",
@@ -7293,13 +7296,15 @@ config_set_allowed_sasl_mechs(const char *attrname, char *value, char *errorbuf 
                           "digits, hyphens, or underscores\n",
                           nval);
             slapi_ch_free_string(&nval);
+            slapi_ch_free_string(&tmp_array_nval);
             return LDAP_UNWILLING_TO_PERFORM;
         }
         CFG_LOCK_WRITE(slapdFrontendConfig);
         slapi_ch_free_string(&slapdFrontendConfig->allowed_sasl_mechs);
         slapi_ch_array_free(slapdFrontendConfig->allowed_sasl_mechs_array);
         slapdFrontendConfig->allowed_sasl_mechs = nval;
-        slapdFrontendConfig->allowed_sasl_mechs_array = slapi_str2charray_ext(nval, " ", 0);
+        slapdFrontendConfig->allowed_sasl_mechs_array = slapi_str2charray_ext(tmp_array_nval, " ", 0);
+        slapi_ch_free_string(&tmp_array_nval);
         CFG_UNLOCK_WRITE(slapdFrontendConfig);
     } else {
         /* If this value is "", we need to set the list to *all* possible mechs */