fix off-by-one in very confusing mech ordering code

[cyrus-sasl.git] / lib / client.c

diff --git a/lib/client.c b/lib/client.c
index 8cc2d1d..d4b37f0 100644 (file)
--- a/lib/client.c
+++ b/lib/client.c
@@ -409,33 +409,45 @@ _sasl_client_order_mechs(const sasl_utils_t *utils,
                         int *server_can_cb)
 {
     char *list, *listp;
-    size_t i;
-    const char *p, *start  = NULL;
+    size_t i, mechslen, start;
 
     *count = 0;
     *server_can_cb = 0;
 
-    listp = list = utils->malloc(strlen(mechs) + 1);
+    if (mechs == NULL || mechs[0] == '\0')
+        return SASL_NOMECH;
+
+    mechslen = strlen(mechs);
+
+    listp = list = utils->malloc(mechslen + 1);
     if (list == NULL)
        return SASL_NOMEM;
 
+    /* xxx confirm this with rfc 2222
+     * SASL mechanism allowable characters are "AZaz-_"
+     * seperators can be any other characters and of any length
+     * even variable lengths between
+     *
+     * Apps should be encouraged to simply use space or comma space
+     * though
+     */
+#define ismechchar(c)   (isalnum((c)) || (c) == '_' || (c) == '-')
     do {
-       for (start = p = mechs, i = 0; *p != '\0'; p++) {
-           if (isspace(*p) || p[1] == '\0') {
-               size_t len = p - start;
-
-               if (p[1] == '\0')
-                   len++;
-
-               if (_mech_plus_p(start, len) == has_cb_data) {
-                   memcpy(listp, start, len);
+        for (i = start = 0; i <= mechslen; i++) {
+           if (!ismechchar(mechs[i])) {
+                const char *mechp = &mechs[start];
+               size_t len = i - start;
+
+               if (len != 0 &&
+                    _mech_plus_p(mechp, len) == has_cb_data) {
+                   memcpy(listp, mechp, len);
                    listp[len] = '\0';
                    listp += len + 1;
                    (*count)++;
                    if (*server_can_cb == 0 && has_cb_data)
                        *server_can_cb = 1;
                }
-               start = p + 1;
+               start = ++i;
            }
        }
        if (has_cb_data)
@@ -444,7 +456,6 @@ _sasl_client_order_mechs(const sasl_utils_t *utils,
            break;
     } while (1);
 
-    *listp = '\0';
     *ordered_mechs = list;
 
     return SASL_OK;
@@ -465,14 +476,6 @@ _sasl_client_order_mechs(const sasl_utils_t *utils,
  *  SASL_INTERACT -- user interaction needed to fill in prompt_need list
  */
 
-/* xxx confirm this with rfc 2222
- * SASL mechanism allowable characters are "AZaz-_"
- * seperators can be any other characters and of any length
- * even variable lengths between
- *
- * Apps should be encouraged to simply use space or comma space
- * though
- */
 int sasl_client_start(sasl_conn_t *conn,
                      const char *mechlist,
                      sasl_interact_t **prompt_need,
@@ -522,7 +525,7 @@ int sasl_client_start(sasl_conn_t *conn,
 
     if (SASL_CB_PRESENT(c_conn->cparams)) {
        if (server_can_cb == 0 && SASL_CB_CRITICAL(c_conn->cparams)) {
-           result = SASL_NOMECH;
+           result = SASL_BADBINDING;
            goto done;
        } else {
            cbindingdisp = SASL_CB_DISP_WANT;