Changeset 3537

Timestamp:

08/15/08 21:00:22 (4 months ago)

Author:

dgollub

Message:

1. The OSyncMessage reply should be checked for null in

opensync_client.c. This patch does that.

2. opensync_client_proxy.c has error_free_message and

error_free_context reversed. So when error_free_context is called
the null message pointer will be dereferenced. Causing a segfault.

3. opensync_queue.c might accidentally dereferenced a null errormsg.

4. In opensync_serializer.c auth is checked when *auth was the pointer

that was actually given the allocated authentication object. Change
the check to *auth.

5. At line 210 of opensync_merger.c new_node is checked for null. Then

at line 213 new_node is dereferenced to get the doc pointer for
the destination doc. Remove call (and commented out call).

6. In opensync_xml.c at line 219 the two pointers are compared. But at

line 223 they are checked for null. Since strcmp will dereference
both pointers to compare their strings this would cause a segfault.
Move the pointer test to before the strcmp.

7. osyncplugin.c checks a pointer but only reports it. So if it is

indeed null a later action might dereference it. The fix is to
error out after reporting.

Signed-off-by: Erik Hovland <erik@…>
Tested-by: Daniel Gollub <dgollub@…>
Acked-by: Daniel Gollub <dgollub@…>

Location:

trunk

Files:

• opensync/client/opensync_client.c (modified) (3 diffs)
• opensync/client/opensync_client_proxy.c (modified) (6 diffs)
• opensync/ipc/opensync_queue.c (modified) (1 diff)
• opensync/ipc/opensync_serializer.c (modified) (1 diff)
• opensync/merger/opensync_merger.c (modified) (1 diff)
• opensync/opensync_xml.c (modified) (1 diff)
• tools/osyncplugin.c (modified) (1 diff)

trunk/opensync/client/opensync_client.c

@@ -115 +115 @@
         if (!osync_error_is_set(&error)) {
                 reply = osync_message_new_reply(message, &locerror);
+                if (!reply)
+                        goto error;
+
                 //Send connect specific reply data
                 osync_message_write_int(reply, slowsync);
@@ -302 +305 @@
         if (!osync_error_is_set(&error)) {
                 reply = osync_message_new_reply(message, &locerror);
+                if (!reply)
+                        goto error;
+
                 //Send get_changes specific reply data
                 osync_message_write_string(reply, osync_change_get_uid(baton->change));
@@ -346 +352 @@
         if (!osync_error_is_set(&error)) {
                 reply = osync_message_new_reply(message, &locerror);
+                if (!reply)
+                        goto error;
+
                 //Send get_changes specific reply data
                 osync_message_write_string(reply, osync_change_get_uid(baton->change));

trunk/opensync/client/opensync_client_proxy.c

@@ -1276 +1276 @@
         return TRUE;
 
+error_free_message:
+        osync_message_unref(message);
 error_free_context:
         g_free(ctx);
-error_free_message:
-        osync_message_unref(message);
 error:
         osync_trace(TRACE_EXIT_ERROR, "%s: %s", __func__, osync_error_print(error));
@@ -1320 +1320 @@
         return TRUE;
 
+error_free_message:
+        osync_message_unref(message);
 error_free_context:
         g_free(ctx);
-error_free_message:
-        osync_message_unref(message);
 error:
         osync_trace(TRACE_EXIT_ERROR, "%s: %s", __func__, osync_error_print(error));
@@ -1364 +1364 @@
         return TRUE;
 
+error_free_message:
+        osync_message_unref(message);
 error_free_context:
         g_free(ctx);
-error_free_message:
-        osync_message_unref(message);
 error:
         osync_trace(TRACE_EXIT_ERROR, "%s: %s", __func__, osync_error_print(error));
@@ -1410 +1410 @@
         return TRUE;
 
+error_free_message:
+        osync_message_unref(message);
 error_free_context:
         g_free(ctx);
-error_free_message:
-        osync_message_unref(message);
 error:
         osync_trace(TRACE_EXIT_ERROR, "%s: %s", __func__, osync_error_print(error));
@@ -1454 +1454 @@
         return TRUE;
 
+error_free_message:
+        osync_message_unref(message);
 error_free_context:
         g_free(ctx);
-error_free_message:
-        osync_message_unref(message);
 error:
         osync_trace(TRACE_EXIT_ERROR, "%s: %s", __func__, osync_error_print(error));
@@ -1498 +1498 @@
         return TRUE;
 
+error_free_message:
+        osync_message_unref(message);
 error_free_context:
         g_free(ctx);
-error_free_message:
-        osync_message_unref(message);
 error:
         osync_trace(TRACE_EXIT_ERROR, "%s: %s", __func__, osync_error_print(error));

trunk/opensync/ipc/opensync_queue.c

@@ -137 +137 @@
 
                         pending->callback(errormsg, pending->user_data);
-                        osync_message_unref(errormsg);
+                        if (errormsg != NULL)
+                                osync_message_unref(errormsg);
 
                         // TODO: Refcounting for OSyncPendingMessage

trunk/opensync/ipc/opensync_serializer.c

@@ -1165 +1165 @@
 
         *auth = osync_plugin_authentication_new(error);
-        if (!auth)
+        if (!*auth)
                 goto error;
 

trunk/opensync/merger/opensync_merger.c

@@ -211 +211 @@
                                                         for(tmp=list; tmp != NULL; tmp = g_slist_next(tmp)) {
                                                                 xmlUnlinkNode(tmp->data);
-                                                                xmlDOMWrapAdoptNode(NULL, ((xmlNodePtr)tmp->data)->doc, tmp->data, new_node->doc, new_par_node, 0);
-                                                                //xmlAddChild(new_par_node, tmp->data);
                                                         }
                                                 }else{

trunk/opensync/opensync_xml.c

@@ -217 +217 @@
                                 goto next;
                         }
+                        if (!leftcontent || !rightcontent) {
+                                osync_trace(TRACE_EXIT, "%s: One is empty", __func__);
+                                return FALSE;
+                        }
                         if (!strcmp(leftcontent, rightcontent)) {
                                 g_free(rightcontent);
                                 goto next;
-                        }
-                        if (!leftcontent || !rightcontent) {
-                                osync_trace(TRACE_EXIT, "%s: One is empty", __func__);
-                                return FALSE;
                         }
                         g_free(rightcontent);

trunk/tools/osyncplugin.c

@@ -1058 +1058 @@
         assert(cmd);
 
-        if (cmd->cmd != CMD_INITIALIZE && *plugin_data == NULL)
+        if (cmd->cmd != CMD_INITIALIZE && *plugin_data == NULL) {
                 fprintf(stderr, "WARNING: Got Plugin initialized? plugin_data is NULL.\n");
+                goto error;
+        }