Fixing 0000280: stack smashing detected

It was a buffer overflow in wd_get_cmd function.

diff --git a/src/include/watchdog/wd_ext.h b/src/include/watchdog/wd_ext.h
index 121cfe039071c631a799b02d9644bbfadde14059..f22e27d04684b427c2db499a11362ed90987cb82 100644 (file)
--- a/src/include/watchdog/wd_ext.h
+++ b/src/include/watchdog/wd_ext.h
@@ -96,7 +96,7 @@ extern bool wd_get_ping_result(char* hostname, int exit_status, int outfd);
 /* wd_if.c */
 extern int wd_IP_up(void);
 extern int wd_IP_down(void);
-extern int wd_get_cmd(char * buf, char * cmd);
+extern char* wd_get_cmd(char* cmd);
 
 /* wd_lifecheck.c */
 extern int is_wd_lifecheck_ready(void);

diff --git a/src/watchdog/watchdog.c b/src/watchdog/watchdog.c
index 92c35f88e319ecc2b1fc87858c596faf630d2280..0ae7f01679a8132ba86ef6091102e0c6c091df43 100644 (file)
--- a/src/watchdog/watchdog.c
+++ b/src/watchdog/watchdog.c
@@ -242,7 +242,7 @@ wd_reaper_watchdog(pid_t pid, bool restart_child)
 void wd_check_network_command_configurations(void)
 {
        char path[128];
-       char cmd[128];
+       char* command;
 
        if (pool_config->use_watchdog == 0)
                return;
@@ -254,38 +254,68 @@ void wd_check_network_command_configurations(void)
                return;
 
        /* check setuid bit of ifup command */
-       wd_get_cmd(cmd, pool_config->if_up_cmd);
-       snprintf(path, sizeof(path), "%s/%s", pool_config->ifconfig_path, cmd);
-       if (! has_setuid_bit(path))
+       command = wd_get_cmd(pool_config->if_up_cmd);
+       if (command)
        {
-               ereport(WARNING,
+               snprintf(path, sizeof(path), "%s/%s", pool_config->ifconfig_path, command);
+               pfree(command);
+               if (! has_setuid_bit(path))
+               {
+                       ereport(WARNING,
                                (errmsg("checking setuid bit of if_up_cmd"),
-                                errdetail("ifup[%s] doesn't have setuid bit", path)));
+                                        errdetail("ifup[%s] doesn't have setuid bit", path)));
+               }
+       }
+       else
+       {
+               ereport(FATAL,
+                       (errmsg("invalid configuration for if_up_cmd parameter"),
+                                errdetail("unable to get command from \"%s\"",pool_config->if_up_cmd)));
        }
        /* check setuid bit of ifdown command */
-       wd_get_cmd(cmd, pool_config->if_down_cmd);
-       snprintf(path, sizeof(path), "%s/%s", pool_config->ifconfig_path, cmd);
-       if (! has_setuid_bit(path))
+       command = wd_get_cmd(pool_config->if_down_cmd);
+       if (command)
        {
-               ereport(WARNING,
-                               (errmsg("checking setuid bit of if_down_cmd"),
-                                errdetail("ifdown[%s] doesn't have setuid bit", path)));
+               snprintf(path, sizeof(path), "%s/%s", pool_config->ifconfig_path, command);
+               pfree(command);
+               if (! has_setuid_bit(path))
+               {
+                       ereport(WARNING,
+                                       (errmsg("checking setuid bit of if_down_cmd"),
+                                        errdetail("ifdown[%s] doesn't have setuid bit", path)));
+               }
+       }
+       else
+       {
+               ereport(FATAL,
+                       (errmsg("invalid configuration for if_down_cmd parameter"),
+                                errdetail("unable to get command from \"%s\"",pool_config->if_down_cmd)));
        }
 
        /* check setuid bit of arping command */
-       wd_get_cmd(cmd, pool_config->arping_cmd);
-       snprintf(path, sizeof(path), "%s/%s", pool_config->arping_path, cmd);
-       if (! has_setuid_bit(path))
+       command = wd_get_cmd(pool_config->arping_cmd);
+       if (command)
        {
-               ereport(WARNING,
-                               (errmsg("checking setuid bit of arping command"),
-                                errdetail("arping[%s] doesn't have setuid bit", path)));
+               snprintf(path, sizeof(path), "%s/%s", pool_config->arping_path, command);
+               pfree(command);
+               if (! has_setuid_bit(path))
+               {
+                       ereport(WARNING,
+                                       (errmsg("checking setuid bit of arping command"),
+                                        errdetail("arping[%s] doesn't have setuid bit", path)));
 
+               }
+       }
+       else
+       {
+               ereport(FATAL,
+                       (errmsg("invalid configuration for arping_cmd parameter"),
+                                errdetail("unable to get command from \"%s\"",pool_config->arping_cmd)));
        }
 }
 
-/* 
- * if the file has setuid bit and the owner is root, it returns 1, otherwise returns 0 
+/*
+ * if the file has setuid bit and the owner is root, it returns 1, otherwise returns 0
  */
 static int
 has_setuid_bit(char * path)

diff --git a/src/watchdog/wd_if.c b/src/watchdog/wd_if.c
index 0f06f675343bdd9d998db69f559e921b971e4308..ad7ca1dd0e171d3fea8ad15c438925cb6c9a326f 100644 (file)
--- a/src/watchdog/wd_if.c
+++ b/src/watchdog/wd_if.c
@@ -47,7 +47,7 @@ wd_IP_up(void)
 {
        int rtn = WD_OK;
        char path[WD_MAX_PATH_LEN];
-       char cmd[128];
+       char* command;
        int i;
 
        if (strlen(pool_config->delegate_IP) == 0)
@@ -57,15 +57,37 @@ wd_IP_up(void)
        {
                WD_List->delegate_ip_flag = 1;
 
-               wd_get_cmd(cmd,pool_config->if_up_cmd);
-               snprintf(path,sizeof(path),"%s/%s",pool_config->ifconfig_path,cmd);
-               rtn = exec_ifconfig(path,pool_config->if_up_cmd);
+               command = wd_get_cmd(pool_config->if_up_cmd);
+               if (command)
+               {
+                       snprintf(path,sizeof(path),"%s/%s",pool_config->ifconfig_path,command);
+                       rtn = exec_ifconfig(path,pool_config->if_up_cmd);
+                       pfree(command);
+               }
+               else
+               {
+                       ereport(LOG,
+                               (errmsg("watchdog failed to bring up delegate IP"),
+                                        errdetail("command not found in if_up_cmd:\"%s\" configuration",pool_config->if_up_cmd)));
+                       rtn = WD_NG;
+               }
 
                if (rtn == WD_OK)
                {
-                       wd_get_cmd(cmd,pool_config->arping_cmd);
-                       snprintf(path,sizeof(path),"%s/%s",pool_config->arping_path,cmd);
-                       rtn = exec_ifconfig(path,pool_config->arping_cmd);
+                       command = wd_get_cmd(pool_config->arping_cmd);
+                       if (command)
+                       {
+                               snprintf(path,sizeof(path),"%s/%s",pool_config->arping_path,command);
+                               rtn = exec_ifconfig(path,pool_config->arping_cmd);
+                               pfree(command);
+                       }
+                       else
+                       {
+                               ereport(LOG,
+                                       (errmsg("watchdog failed to bring up delegate IP"),
+                                                errdetail("command not found in arping_cmd:\"%s\" configuration",pool_config->arping_cmd)));
+                               rtn = WD_NG;
+                       }
                }
                if (rtn == WD_OK)
                {
@@ -105,7 +127,7 @@ wd_IP_down(void)
 {
        int rtn = WD_OK;
        char path[WD_MAX_PATH_LEN];
-       char cmd[128];
+       char* command;
        int i;
 
        if (strlen(pool_config->delegate_IP) == 0)
@@ -114,9 +136,20 @@ wd_IP_down(void)
        if (WD_List->delegate_ip_flag == 1)
        {
                WD_List->delegate_ip_flag = 0;
-               wd_get_cmd(cmd,pool_config->if_down_cmd);
-               snprintf(path, sizeof(path), "%s/%s", pool_config->ifconfig_path, cmd);
-               rtn = exec_ifconfig(path,pool_config->if_down_cmd);
+               command = wd_get_cmd(pool_config->if_down_cmd);
+               if (command)
+               {
+                       snprintf(path, sizeof(path), "%s/%s", pool_config->ifconfig_path, command);
+                       rtn = exec_ifconfig(path,pool_config->if_down_cmd);
+                       pfree(command);
+               }
+               else
+               {
+                       ereport(LOG,
+                                       (errmsg("watchdog failed to bring down delegate IP"),
+                                        errdetail("command not found in if_down_cmd:\"%s\" configuration",pool_config->if_down_cmd)));
+                       return WD_NG;
+               }
 
                if (rtn == WD_OK)
                {
@@ -153,22 +186,20 @@ wd_IP_down(void)
        return rtn;
 }
 
-int
-wd_get_cmd(char * buf, char * cmd)
+char*
+wd_get_cmd(char* cmd)
 {
-       int i,j;
-       i = 0;
-       while(isspace(cmd[i]) != 0)
-       {
-               i++;
-       }
-       j = 0;
-       while(isspace(cmd[i]) == 0)
+       char *command = NULL;
+
+       if (cmd && *cmd)
        {
-               buf[j++] = cmd[i++];
+               char* tmp_str = pstrdup(cmd);
+               char* token = strtok(tmp_str," ");
+               if (token)
+                       command = pstrdup(token);
+               pfree(tmp_str);
        }
-       buf[j] = '\0';
-       return strlen(buf);
+       return command;
 }
 
 static int