amoralej / rpms / 389-ds-base

Forked from rpms/389-ds-base 5 years ago
Clone
Blob Blame History Raw
From fe0ac5946b04d9ff2455692ddb8c0a8b0c91a7c7 Mon Sep 17 00:00:00 2001
From: Noriko Hosoi <nhosoi@redhat.com>
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