From 6b256ffdb4b179a253a8cc55973314a6990985a0 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Sun, 11 Aug 2013 12:52:56 +0200 Subject: [PATCH 1/2] toxic: Fix get_user_config_dir(). --- testing/toxic/configdir.c | 114 ++++++++++++++++++++++---------------- 1 file changed, 66 insertions(+), 48 deletions(-) diff --git a/testing/toxic/configdir.c b/testing/toxic/configdir.c index 6cbb06bc..18e211ce 100644 --- a/testing/toxic/configdir.c +++ b/testing/toxic/configdir.c @@ -28,65 +28,83 @@ #ifdef WIN32 #include #include -#endif - -#ifdef __APPLE__ +#else /* WIN32 */ #include #include -#endif +#endif /* WIN32 */ #include "configdir.h" -/* - * Retrieves a correct configuration directory, depending on the OS used, with a trailing slash +/** + * @brief Get the users config directory. + * + * This is without a trailing slash. + * + * @return The users config dir or NULL on error. */ char *get_user_config_dir(void) { - char *user_config_dir; + char *user_config_dir; +#ifdef WIN32 + char appdata[MAX_PATH]; + BOOL ok; - #ifdef WIN32 - - char appdata[MAX_PATH]; - HRESULT result = SHGetFolderPath( - NULL, - CSIDL_APPDATA, - NULL, - SHGFP_TYPE_CURRENT, - appdata - ) - if (!result) return NULL; - - user_config_dir = strdup(appdata); - - return user_config_dir; - - #elif defined __APPLE__ - - struct passwd *pass = getpwuid(getuid()); - if (!pass) return NULL; - char *home = pass->pw_dir; - user_config_dir = malloc(strlen(home) + strlen("/Library/Application Support") + 1); - - if(user_config_dir) { - strcpy(user_config_dir, home); - strcat(user_config_dir, "/Library/Application Support"); - } - return user_config_dir; - - #else - - if (getenv("XDG_CONFIG_HOME")) { - user_config_dir = strdup(getenv("XDG_CONFIG_HOME")); - } else { - user_config_dir = malloc(strlen(getenv("HOME")) + strlen("/.config") + 1); - if (user_config_dir) { - strcpy(user_config_dir, getenv("HOME")); - strcat(user_config_dir, "/.config"); + ok = SHGetSpecialFolderPathA(NULL, appdata, CSIDL_PROFILE, TRUE); + if (!ok) { + return NULL; } - } - return user_config_dir; - #endif + user_config_dir = strdup(appdata); + + return user_config_dir; + +#else /* WIN32 */ + +#ifndef NSS_BUFLEN_PASSWD +#define NSS_BUFLEN_PASSWD 4096 +#endif /* NSS_BUFLEN_PASSWD */ + + struct passwd pwd; + struct passwd *pwdbuf; + const char *home; + char buf[NSS_BUFLEN_PASSWD]; + size_t len; + int rc; + + rc = getpwuid_r(getuid(), &pwd, buf, NSS_BUFLEN_PASSWD, &pwdbuf); + if (rc == 0) { + home = pwd.pw_dir; + } else { + home = getenv("HOME"); + if (home == NULL) { + return NULL; + } + /* env variables can be tainted */ + snprintf(buf, sizeof(buf), "%s", home); + home = buf; + } + +# if defined(__APPLE__) + len = strlen(home) + strlen("/Library/Application Support") + 1; + user_config_dir = malloc(len); + if (user_config_dir == NULL) { + return NULL; + } + + snprintf(user_config_dir, len, "%s/Library/Application Support", home); +# else /* __APPLE__ */ + len = strlen(home) + strlen("/.config") + 1; + user_config_dir = malloc(len); + if (user_config_dir == NULL) { + return NULL; + } + + snprintf(user_config_dir, len, "%s/.config", home); +# endif /* __APPLE__ */ + + return user_config_dir; +#undef NSS_BUFLEN_PASSWD +#endif /* WIN32 */ } /* From 6b06431e9bcbef2eb1126dda01a68d4a81f0825e Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Sun, 11 Aug 2013 15:24:47 +0200 Subject: [PATCH 2/2] core: Fix a possible buffer overflow using getself_name(). If the passed buffer is smaller than MAX_NAME_LENGTH then, you will probably overflow it. --- auto_tests/messenger_test.c | 2 +- core/Messenger.c | 12 ++++++++++-- core/Messenger.h | 16 ++++++++++++---- testing/nTox.c | 2 +- testing/toxic/chat.c | 2 +- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/auto_tests/messenger_test.c b/auto_tests/messenger_test.c index 64b44d5f..28747899 100644 --- a/auto_tests/messenger_test.c +++ b/auto_tests/messenger_test.c @@ -169,7 +169,7 @@ START_TEST(test_getself_name) char nick_check[len]; setname(m, (uint8_t *)nickname, len); - getself_name(m, (uint8_t *)nick_check); + getself_name(m, (uint8_t *)nick_check, len); ck_assert_msg((!STRINGS_EQUAL(nickname, nick_check)), "getself_name failed to return the known name!\n" diff --git a/core/Messenger.c b/core/Messenger.c index ebde5a78..1c81163c 100644 --- a/core/Messenger.c +++ b/core/Messenger.c @@ -267,10 +267,18 @@ int setname(Messenger *m, uint8_t * name, uint16_t length) put it in name name needs to be a valid memory location with a size of at least MAX_NAME_LENGTH bytes. return the length of the name */ -uint16_t getself_name(Messenger *m, uint8_t *name) +uint16_t getself_name(Messenger *m, uint8_t *name, uint16_t nlen) { + uint16_t len; + + if (name == NULL || nlen == 0) { + return 0; + } + + len = MIN(nlen, m->name_length); memcpy(name, m->name, m->name_length); - return m->name_length; + + return len; } /* get name of friendnumber diff --git a/core/Messenger.h b/core/Messenger.h index fa69d104..aa9611a4 100644 --- a/core/Messenger.h +++ b/core/Messenger.h @@ -196,10 +196,18 @@ int m_sendaction(Messenger *m, int friendnumber, uint8_t *action, uint32_t lengt return -1 if failure */ int setname(Messenger *m, uint8_t *name, uint16_t length); -/* get our nickname - put it in name - return the length of the name*/ -uint16_t getself_name(Messenger *m, uint8_t *name); +/** + * @brief Get your nickname. + * + * @param[in] m The messanger context to use. + * + * @param[inout] name Pointer to a string for the name. + * + * @param[in] nlen The length of the string buffer. + * + * @return Return the length of the name, 0 on error. + */ +uint16_t getself_name(Messenger *m, uint8_t *name, uint16_t nlen); /* get name of friendnumber put it in name diff --git a/testing/nTox.c b/testing/nTox.c index 59d1cbf6..74db2ae2 100644 --- a/testing/nTox.c +++ b/testing/nTox.c @@ -113,7 +113,7 @@ char *format_message(Messenger *m, char *message, int friendnum) if (friendnum != -1) { getname(m, friendnum, (uint8_t*)name); } else { - getself_name(m, (uint8_t*)name); + getself_name(m, (uint8_t*)name, sizeof(name)); } char *msg = malloc(100+strlen(message)+strlen(name)+1); diff --git a/testing/toxic/chat.c b/testing/toxic/chat.c index 112b20b7..59b13492 100644 --- a/testing/toxic/chat.c +++ b/testing/toxic/chat.c @@ -210,7 +210,7 @@ void execute(ToxWindow *self, ChatContext *ctx, Messenger *m, char *cmd, struct wattroff(ctx->history, COLOR_PAIR(2)); uint8_t selfname[MAX_NAME_LENGTH]; - int len = getself_name(m, selfname); + int len = getself_name(m, selfname, sizeof(selfname)); char msg[MAX_STR_SIZE-len-4]; snprintf(msg, sizeof(msg), "* %s %s\n", (uint8_t*) selfname, action);