Clean up string handling

Revise the handling of strings in several places to be less confusing
and safer.  Fix at least two definite bugs in launcher.c concerning
integer overflows and realloc() possibly moving memory.

Also a few indentation changes.

diff --git a/dbus-server-bindings.c b/dbus-server-bindings.c
index c610881..3c7718e 100644 (file)
--- a/dbus-server-bindings.c
+++ b/dbus-server-bindings.c
@@ -51,8 +51,7 @@ static void open_address(const char *uri) {
                new_uri_len = strlen("file://") + strlen(uri) + 1;
                if (!(new_uri = calloc(new_uri_len, sizeof(char))))
                        exit(1);
-               strncpy(new_uri, "file://", strlen("file://"));
-               strncat(new_uri, uri, strlen(uri));
+               snprintf(new_uri, new_uri_len, "%s%s", "file://", uri);
 
                launch_browser(&ctx, new_uri);
                /* If launch_browser didn't exec something in this process,
@@ -110,11 +109,11 @@ void dbus_request_osso_browser_name(struct swb_context *ctx) {
                return;
 
        if (!dbus_g_proxy_call(ctx->dbus_proxy, "RequestName", &error,
-                               G_TYPE_STRING, "com.nokia.osso_browser",
-                               G_TYPE_UINT, DBUS_NAME_FLAG_REPLACE_EXISTING|DBUS_NAME_FLAG_DO_NOT_QUEUE,
-                               G_TYPE_INVALID,
-                               G_TYPE_UINT, &result,
-                               G_TYPE_INVALID)) {
+                              G_TYPE_STRING, "com.nokia.osso_browser",
+                              G_TYPE_UINT, DBUS_NAME_FLAG_REPLACE_EXISTING|DBUS_NAME_FLAG_DO_NOT_QUEUE,
+                              G_TYPE_INVALID,
+                              G_TYPE_UINT, &result,
+                              G_TYPE_INVALID)) {
                printf("Couldn't acquire name com.nokia.osso_browser\n");
                exit(1);
        }
@@ -133,8 +132,8 @@ void dbus_release_osso_browser_name(struct swb_context *ctx) {
                return;
 
        dbus_g_proxy_call(ctx->dbus_proxy, "ReleaseName", &error,
-                               G_TYPE_STRING, "com.nokia.osso_browser",
-                               G_TYPE_INVALID,
-                               G_TYPE_UINT, &result,
-                               G_TYPE_INVALID);
+                         G_TYPE_STRING, "com.nokia.osso_browser",
+                         G_TYPE_INVALID,
+                         G_TYPE_UINT, &result,
+                         G_TYPE_INVALID);
 }

diff --git a/launcher.c b/launcher.c
index c897299..5a6b951 100644 (file)
--- a/launcher.c
+++ b/launcher.c
@@ -53,10 +53,8 @@ static void launch_tear(struct swb_context *ctx, char *uri) {
                        tear_proxy = dbus_g_proxy_new_for_name(ctx->session_bus,
                                        "com.nokia.tear", "/com/nokia/tear",
                                        "com.nokia.Tear");
-               dbus_g_proxy_call(tear_proxy, "OpenAddress",
-                               &error,
-                               G_TYPE_STRING, uri,
-                               G_TYPE_INVALID);
+               dbus_g_proxy_call(tear_proxy, "OpenAddress", &error,
+                                 G_TYPE_STRING, uri, G_TYPE_INVALID);
                if (!ctx->continuous_mode)
                        exit(0);
        } else {
@@ -102,10 +100,10 @@ void launch_microb(struct swb_context *ctx, char *uri) {
                /* Child process */
                if (!strcmp(uri, "new_window")) {
                        execl("/usr/bin/maemo-invoker",
-                                      "browser", (char *)NULL);
+                             "browser", (char *)NULL);
                } else {
                        execl("/usr/bin/maemo-invoker",
-                                       "browser", "--url", uri, (char *)NULL);
+                             "browser", "--url", uri, (char *)NULL);
                }
        }
 
@@ -123,6 +121,8 @@ static void launch_other_browser(struct swb_context *ctx, char *uri) {
        char *quoted_uri, *quote;
 
        size_t cmdlen, urilen;
+       size_t quoted_uri_size;
+       size_t offset;
 
        if (!uri || !strcmp(uri, "new_window"))
                uri = "";
@@ -132,20 +132,30 @@ static void launch_other_browser(struct swb_context *ctx, char *uri) {
                /* urilen+3 = length of URI + 2x \' + \0 */
                if (!(quoted_uri = calloc(urilen+3, sizeof(char))))
                        exit(1);
-               strncpy(quoted_uri+1, uri, urilen);
-               quoted_uri[0] = quoted_uri[urilen+1] = '\'';
-               /* calloc zeroes the memory, so string is automatically
-                  null terminated */
+               snprintf(quoted_uri, urilen+3, "'%s'", uri);
 
                /* If there are any 's in the original URI, URL-escape them
                   (replace them with %27) */
+               quoted_uri_size = urilen + 3;
                quote = quoted_uri + 1;
                while ((quote = strchr(quote, '\'')) &&
-                               (quote-quoted_uri) < strlen(quoted_uri)-1) {
-                       /* 3 = strlen("%27")-strlen("'") + \0 */
+                      (offset = quote-quoted_uri) < strlen(quoted_uri)-1) {
+                       /* Check to make sure we don't shrink the memory area
+                          as a result of integer overflow */
+                       if (quoted_uri_size+2 <= quoted_uri_size)
+                               exit(1);
+
+                       /* Grow the memory area;
+                          2 = strlen("%27")-strlen("'") */
                        if (!(quoted_uri = realloc(quoted_uri,
-                                                       strlen(quoted_uri)+3)))
+                                                  quoted_uri_size+2)))
                                exit(1);
+                       quoted_uri_size = quoted_uri_size + 2;
+
+                       /* Recalculate the location of the ' character --
+                          realloc() may have moved the string in memory */
+                       quote = quoted_uri + offset;
+
                        /* Move the string after the ', including the \0,
                           over two chars */
                        memmove(quote+3, quote+1, strlen(quote)+1);
@@ -190,8 +200,7 @@ static void use_other_browser_cmd(struct swb_context *ctx, char *cmd) {
                ctx->default_browser_launcher = LAUNCH_DEFAULT_BROWSER;
        } else {
                ctx->other_browser_cmd = strncpy(ctx->other_browser_cmd,
-                               cmd, len);
-               ctx->other_browser_cmd[len] = '\0';
+                                                cmd, len+1);
                ctx->default_browser_launcher = launch_other_browser;
        }
 }

diff --git a/main.c b/main.c
index 428d5bc..5403b70 100644 (file)
--- a/main.c
+++ b/main.c
@@ -71,8 +71,7 @@ static void read_config(int signalnum) {
        len = strlen(homedir) + strlen(CONFIGFILE_LOC) + 1;
        if (!(configfile = calloc(len, sizeof(char))))
                goto out_noopen;
-       strncpy(configfile, homedir, strlen(homedir));
-       strncat(configfile, CONFIGFILE_LOC, strlen(CONFIGFILE_LOC));
+       snprintf(configfile, len, "%s%s", homedir, CONFIGFILE_LOC);
 
        if (!(fp = fopen(configfile, "r")))
                goto out_noopen;