From fe0ac5946b04d9ff2455692ddb8c0a8b0c91a7c7 Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Mon, 3 Nov 2014 16:58:21 -0800 Subject: [PATCH 26/28] Ticket #47939 - Malformed cookie for LDAP Sync makes DS crash Bug Description: If a cookie sent from clients did not have the expected form: server_signature#client_signature#change_info_number, a NULL reference triggered a server crash in sync_cookie_isvalid. Fix Description: If a cookie does not have the expected form, sync_cookie_parse returns NULL, which prevents the NULL reference in the server_signature and client_signature. https://fedorahosted.org/389/ticket/47939 Reviewed by lkrispen@redhat.com (Thank you, Ludwig!!) (cherry picked from commit 8f540a6cee09be13430ebe0b983d2affe2863365) (cherry picked from commit d87202acad6426bee7af8753a0ffe5ad5b3082df) --- ldap/servers/plugins/sync/sync_util.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/ldap/servers/plugins/sync/sync_util.c b/ldap/servers/plugins/sync/sync_util.c index ef4a3f7..de65b99 100644 --- a/ldap/servers/plugins/sync/sync_util.c +++ b/ldap/servers/plugins/sync/sync_util.c @@ -552,21 +552,21 @@ Sync_Cookie * sync_cookie_parse (char *cookie) { char *p, *q; - Sync_Cookie *sc; + Sync_Cookie *sc = NULL; if (cookie == NULL || *cookie == '\0' ) { return NULL; } + /* + * Format of cookie: server_signature#client_signature#change_info_number + * If the cookie is malformed, NULL is returned. + */ p = q = cookie; - sc = (Sync_Cookie *)slapi_ch_malloc(sizeof(Sync_Cookie)); - - sc->cookie_client_signature = NULL; - sc->cookie_server_signature = NULL; - sc->cookie_change_info = -1; p = strchr(q, '#'); if (p) { *p = '\0'; + sc = (Sync_Cookie *)slapi_ch_calloc(1, sizeof(Sync_Cookie)); sc->cookie_server_signature = slapi_ch_strdup(q); q = p + 1; p = strchr(q, '#'); @@ -574,21 +574,32 @@ sync_cookie_parse (char *cookie) *p = '\0'; sc->cookie_client_signature = slapi_ch_strdup(q); sc->cookie_change_info = sync_number2int(p+1); + if (sc->cookie_change_info < 0) { + goto error_return; + } + } else { + goto error_return; } } - return (sc); +error_return: + slapi_ch_free_string(&(sc->cookie_client_signature)); + slapi_ch_free_string(&(sc->cookie_server_signature)); + slapi_ch_free((void **)&sc); + return NULL; } int sync_cookie_isvalid (Sync_Cookie *testcookie, Sync_Cookie *refcookie) { /* client and server info must match */ - if (strcmp(testcookie->cookie_client_signature,refcookie->cookie_client_signature) || - strcmp(testcookie->cookie_server_signature,refcookie->cookie_server_signature) || - testcookie->cookie_change_info == -1 || - testcookie->cookie_change_info > refcookie->cookie_change_info ) + if ((testcookie && refcookie) && + (strcmp(testcookie->cookie_client_signature,refcookie->cookie_client_signature) || + strcmp(testcookie->cookie_server_signature,refcookie->cookie_server_signature) || + testcookie->cookie_change_info == -1 || + testcookie->cookie_change_info > refcookie->cookie_change_info)) { return (0); + } /* could add an additional check if the requested state in client cookie is still * available. Accept any state request for now. */ -- 1.9.3