[BUG] str2sun could leak a small buffer in case of error during parsing

Matt Farnsworth reported a memory leak in str2sun() in case a too large
socket path is passed. The bug is very minor because it only happens
once during config parsing, but has to be fixed nevertheless. The patch
Matt provided could even be improved by completely removing the useless
strdup() in this function.

diff --git a/include/common/standard.h b/include/common/standard.h
index 162a9ebc09b2494bba5516cc89b6fab9d4b5e0cb..a120f56bd46db38c020cf3d32cf4296fd1bb91ec 100644 (file)
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -139,7 +139,7 @@ extern const char *invalid_char(const char *name);
  * converts <str> to a struct sockaddr_un* which is locally allocated.
  * The format is "/path", where "/path" is a path to a UNIX domain socket.
  */
-struct sockaddr_un *str2sun(char *str);
+struct sockaddr_un *str2sun(const char *str);
 
 /*
  * converts <str> to a struct sockaddr_in* which is locally allocated.

diff --git a/src/standard.c b/src/standard.c
index 93cf1f89e5d9a5eb4bfbab87a0d6cb2e333cd817..7f749f957505a0517f9100250f80e2e644588c9f 100644 (file)
--- a/src/standard.c
+++ b/src/standard.c
@@ -83,27 +83,20 @@ const char *limit_r(unsigned long n, char *buffer, int size, const char *alt)
  * converts <str> to a struct sockaddr_un* which is locally allocated.
  * The format is "/path", where "/path" is a path to a UNIX domain socket.
  */
-struct sockaddr_un *str2sun(char *str)
+struct sockaddr_un *str2sun(const char *str)
 {
        static struct sockaddr_un su;
        int strsz;      /* length included null */
 
        memset(&su, 0, sizeof(su));
-       str = strdup(str);
-       if (str == NULL)
-               goto out_nofree;
-
        strsz = strlen(str) + 1;
        if (strsz > sizeof(su.sun_path)) {
                Alert("Socket path '%s' too long (max %d)\n",
                        str, sizeof(su.sun_path) - 1);
-               goto out_nofree;
+       } else {
+               su.sun_family = AF_UNIX;
+               memcpy(su.sun_path, str, strsz);
        }
-       su.sun_family = AF_UNIX;
-       memcpy(su.sun_path, str, strsz);
-
-       free(str);
- out_nofree:
        return &su;
 }