safer string handling

src/chat.c

@@ -255,18 +255,13 @@ static void chat_onNickChange(ToxWindow *self, Tox *m, int32_t num, const char *
     if (self->num != num)
         return;
 
-    if (len > TOX_MAX_NAME_LENGTH)
-        return;
-
     StatusBar *statusbar = self->stb;
 
-    char tmpname[TOX_MAX_NAME_LENGTH];
+    snprintf(statusbar->nick, sizeof(statusbar->nick), "%s", nick);
-    strcpy(tmpname, nick);
+    len = strlen(statusbar->nick);
-    int n_len = MIN(len, TOXIC_MAX_NAME_LENGTH - 1);
+    statusbar->nick_len = len;
-    tmpname[n_len] = '\0';
 
-    snprintf(statusbar->nick, sizeof(statusbar->nick), "%s", tmpname);
+    chat_set_window_name(self, statusbar->nick, len);
-    chat_set_window_name(self, tmpname, n_len);
 }
 
 static void chat_onStatusChange(ToxWindow *self, Tox *m, int32_t num, uint8_t status)
@@ -581,7 +576,7 @@ static void chat_onGroupInvite(ToxWindow *self, Tox *m, int32_t friendnumber, co
     Friends.list[friendnumber].group_invite.pending = true;
     Friends.list[friendnumber].group_invite.length = length;
 
-    char name[TOX_MAX_NAME_LENGTH];
+    char name[TOX_MAX_NAME_LENGTH + 1];
     get_nick_truncate(m, name, friendnumber);
 
     sound_notify(self, generic_message, NT_WNDALERT_2, NULL);
@@ -1087,7 +1082,7 @@ static void chat_onInit(ToxWindow *self, Tox *m)
     snprintf(statusbar->statusmsg, sizeof(statusbar->statusmsg), "%s", statusmsg);
     statusbar->statusmsg_len = s_len;
 
-    char nick[TOX_MAX_NAME_LENGTH];
+    char nick[TOX_MAX_NAME_LENGTH + 1];
     int n_len = get_nick_truncate(m, nick, self->num);
     snprintf(statusbar->nick, sizeof(statusbar->nick), "%s", nick);
     statusbar->nick_len = n_len;

src/dns.c

@@ -47,7 +47,7 @@
 #define TOX_DNS3_TXT_PREFIX "v=tox3;id="
 
 extern struct Winthread Winthread;
-extern struct _dns3_servers dns3_servers;
+extern struct dns3_servers dns3_servers;
 extern struct arg_opts arg_opts;
 
 #define NUM_DNS3_BACKUP_SERVERS 2
@@ -73,7 +73,7 @@ static struct dns3_server_backup {
     },
 };
 
-static struct _thread_data {
+static struct thread_data {
     ToxWindow *self;
     char id_bin[TOX_FRIEND_ADDRESS_SIZE];
     char addr[MAX_STR_SIZE];
@@ -82,7 +82,7 @@ static struct _thread_data {
     Tox *m;
 } t_data;
 
-static struct _dns_thread {
+static struct dns_thread {
     pthread_t tid;
     pthread_attr_t attr;
 } dns_thread;
@@ -92,7 +92,7 @@ static struct _dns_thread {
 #define MAX_DOMAIN_SIZE 32
 #define MAX_DNS_LINE MAX_DOMAIN_SIZE + (DNS3_KEY_SIZE * 2) + 3
 
-struct _dns3_servers {
+struct dns3_servers {
     bool loaded;
     int lines;
     char names[MAX_DNS_SERVERS][MAX_DOMAIN_SIZE];
@@ -152,12 +152,12 @@ static int dns_error(ToxWindow *self, const char *errmsg)
     return -1;
 }
 
-static void kill_dns_thread(void *dns_obj)
+static void killdns_thread(void *dns_obj)
 {
     if (dns_obj)
         tox_dns3_kill(dns_obj);
 
-    memset(&t_data, 0, sizeof(struct _thread_data));
+    memset(&t_data, 0, sizeof(struct thread_data));
     pthread_attr_destroy(&dns_thread.attr);
     pthread_exit(NULL);
 }
@@ -299,7 +299,7 @@ void *dns3_lookup_thread(void *data)
 
     if (namelen == -1) {
         dns_error(self, "Must be a Tox ID or an address in the form username@domain");
-        kill_dns_thread(NULL);
+        killdns_thread(NULL);
     }
 
     char DNS_pubkey[DNS3_KEY_SIZE];
@@ -309,14 +309,14 @@ void *dns3_lookup_thread(void *data)
 
     if (match == -1) {
         dns_error(self, "Domain not found.");
-        kill_dns_thread(NULL);
+        killdns_thread(NULL);
     }
 
     void *dns_obj = tox_dns3_new((uint8_t *) DNS_pubkey);
 
     if (dns_obj == NULL) {
         dns_error(self, "Core failed to create DNS object.");
-        kill_dns_thread(NULL);
+        killdns_thread(NULL);
     }
 
     char string[MAX_DNS_REQST_SIZE + 1];
@@ -327,7 +327,7 @@ void *dns3_lookup_thread(void *data)
 
     if (str_len == -1) {
         dns_error(self, "Core failed to generate DNS3 string.");
-        kill_dns_thread(dns_obj);
+        killdns_thread(dns_obj);
     }
 
     string[str_len] = '\0';
@@ -341,14 +341,14 @@ void *dns3_lookup_thread(void *data)
 
     if (ans_len <= 0) {
         dns_error(self, "DNS query failed.");
-        kill_dns_thread(dns_obj);
+        killdns_thread(dns_obj);
     }
 
     char ans_id[MAX_DNS_REQST_SIZE + 1];
 
     /* extract TXT from DNS response */
     if (parse_dns_response(self, answer, ans_len, ans_id) == -1)
-        kill_dns_thread(dns_obj);
+        killdns_thread(dns_obj);
 
     char encrypted_id[MAX_DNS_REQST_SIZE + 1];
     int prfx_len = strlen(TOX_DNS3_TXT_PREFIX);
@@ -356,7 +356,7 @@ void *dns3_lookup_thread(void *data)
     /* extract the encrypted ID from TXT response */
     if (strncmp(ans_id, TOX_DNS3_TXT_PREFIX, prfx_len) != 0) {
         dns_error(self, "Bad DNS3 TXT response.");
-        kill_dns_thread(dns_obj);
+        killdns_thread(dns_obj);
     }
 
     memcpy(encrypted_id, ans_id + prfx_len, ans_len - prfx_len);
@@ -364,14 +364,14 @@ void *dns3_lookup_thread(void *data)
     if (tox_decrypt_dns3_TXT(dns_obj, (uint8_t *) t_data.id_bin, (uint8_t *) encrypted_id, 
                              strlen(encrypted_id), request_id) == -1) {
         dns_error(self, "Core failed to decrypt DNS response.");
-        kill_dns_thread(dns_obj);
+        killdns_thread(dns_obj);
     }
 
     pthread_mutex_lock(&Winthread.lock);
     cmd_add_helper(self, t_data.m, t_data.id_bin, t_data.msg);
     pthread_mutex_unlock(&Winthread.lock);
 
-    kill_dns_thread(dns_obj);
+    killdns_thread(dns_obj);
     return 0;
 }
 

src/friendlist.c

@@ -340,25 +340,21 @@ static void friendlist_onConnectionChange(ToxWindow *self, Tox *m, int32_t num,
 
 static void friendlist_onNickChange(ToxWindow *self, Tox *m, int32_t num, const char *nick, uint16_t len)
 {
-    if (len > TOX_MAX_NAME_LENGTH || num >= Friends.max_idx)
+    if (num >= Friends.max_idx)
         return;
 
     /* save old name for log renaming */
-    char oldname[TOXIC_MAX_NAME_LENGTH];
+    char oldname[TOXIC_MAX_NAME_LENGTH + 1];
     snprintf(oldname, sizeof(oldname), "%s", Friends.list[num].name);
 
     /* update name */
-    char tempname[TOX_MAX_NAME_LENGTH];
+    snprintf(Friends.list[num].name, sizeof(Friends.list[num].name), "%s", nick);
-    strcpy(tempname, nick);
+    Friends.list[num].namelength = strlen(Friends.list[num].name);
-    len = MIN(len, TOXIC_MAX_NAME_LENGTH - 1);
-    tempname[len] = '\0';
-    snprintf(Friends.list[num].name, sizeof(Friends.list[num].name), "%s", tempname);
-    Friends.list[num].namelength = len;
 
     /* get data for chatlog renaming */
-    char newnamecpy[TOXIC_MAX_NAME_LENGTH];
+    char newnamecpy[TOXIC_MAX_NAME_LENGTH + 1];
     char myid[TOX_FRIEND_ADDRESS_SIZE];
-    strcpy(newnamecpy, tempname);
+    strcpy(newnamecpy, Friends.list[num].name);
     tox_get_address(m, (uint8_t *) myid);
 
     if (strcmp(oldname, newnamecpy) != 0)

src/friendlist.h

@@ -55,9 +55,9 @@ struct GroupChatInvite {
 };
 
 typedef struct {
-    char name[TOXIC_MAX_NAME_LENGTH];
+    char name[TOXIC_MAX_NAME_LENGTH + 1];
     int namelength;
-    char statusmsg[TOX_MAX_STATUSMESSAGE_LENGTH];
+    char statusmsg[TOX_MAX_STATUSMESSAGE_LENGTH + 1];
     uint16_t statusmsg_len;
     char pub_key[TOX_CLIENT_ID_SIZE];
     int32_t num;
@@ -74,7 +74,7 @@ typedef struct {
 } ToxicFriend;
 
 typedef struct {
-    char name[TOXIC_MAX_NAME_LENGTH];
+    char name[TOXIC_MAX_NAME_LENGTH + 1];
     int namelength;
     char pub_key[TOX_CLIENT_ID_SIZE];
     int32_t num;

src/groupchat.c

@@ -134,11 +134,6 @@ static void groupchat_onGroupMessage(ToxWindow *self, Tox *m, int groupnum, int
     if (self->num != groupnum)
         return;
 
-    char msg_cpy[MAX_LINE_INFO_MSG_SIZE];
-    len = MIN(len, MAX_LINE_INFO_MSG_SIZE - 1);
-    memcpy(&msg_cpy, msg, len);
-    msg_cpy[len] = '\0';
-
     ChatContext *ctx = self->chatwin;
 
     char nick[TOX_MAX_NAME_LENGTH];
@@ -153,13 +148,13 @@ static void groupchat_onGroupMessage(ToxWindow *self, Tox *m, int groupnum, int
     int nick_clr = strcmp(nick, selfnick) == 0 ? GREEN : CYAN;
 
     /* Only play sound if mentioned */
-    if (strcasestr(msg_cpy, selfnick) && strncmp(selfnick, nick, TOXIC_MAX_NAME_LENGTH - 1)) {
+    if (strcasestr(msg, selfnick)) {
         sound_notify(self, generic_message, NT_WNDALERT_0, NULL);
                 
         if (self->active_box != -1)
-            box_silent_notify2(self, NT_NOFOCUS, self->active_box, "%s %s", nick, msg_cpy);
+            box_silent_notify2(self, NT_NOFOCUS, self->active_box, "%s %s", nick, msg);
         else
-            box_silent_notify(self, NT_NOFOCUS, &self->active_box, self->name, "%s %s", nick, msg_cpy);
+            box_silent_notify(self, NT_NOFOCUS, &self->active_box, self->name, "%s %s", nick, msg);
 
         nick_clr = RED;
     }
@@ -170,8 +165,8 @@ static void groupchat_onGroupMessage(ToxWindow *self, Tox *m, int groupnum, int
     char timefrmt[TIME_STR_SIZE];
     get_time_str(timefrmt, sizeof(timefrmt));
 
-    line_info_add(self, timefrmt, nick, NULL, IN_MSG, 0, nick_clr, "%s", msg_cpy);
+    line_info_add(self, timefrmt, nick, NULL, IN_MSG, 0, nick_clr, "%s", msg);
-    write_to_log(msg_cpy, nick, ctx->log, false);
+    write_to_log(msg, nick, ctx->log, false);
 }
 
 static void groupchat_onGroupAction(ToxWindow *self, Tox *m, int groupnum, int peernum, const char *action,
@@ -199,7 +194,8 @@ static void groupchat_onGroupAction(ToxWindow *self, Tox *m, int groupnum, int p
         else
             box_silent_notify(self, NT_NOFOCUS, &self->active_box, self->name, "* %s %s", nick, action);
     }
-    else sound_notify(self, silent, NT_WNDALERT_1, NULL);
+    else 
+        sound_notify(self, silent, NT_WNDALERT_1, NULL);
 
     char nick[TOX_MAX_NAME_LENGTH];
     n_len = tox_group_peername(m, groupnum, peernum, (uint8_t *) nick);

src/line_info.h

@@ -46,8 +46,8 @@ enum {
 
 struct line_info {
     char timestr[TIME_STR_SIZE];
-    char name1[TOXIC_MAX_NAME_LENGTH];
+    char name1[TOXIC_MAX_NAME_LENGTH + 1];
-    char name2[TOXIC_MAX_NAME_LENGTH];
+    char name2[TOXIC_MAX_NAME_LENGTH + 1];
     char msg[MAX_LINE_INFO_MSG_SIZE];
     uint64_t timestamp;
     uint8_t type;

src/misc_tools.c

@@ -250,11 +250,21 @@ void str_to_lower(char *str)
 int get_nick_truncate(Tox *m, char *buf, int friendnum)
 {
     int len = tox_get_name(m, friendnum, (uint8_t *) buf);
-    len = MIN(len, TOXIC_MAX_NAME_LENGTH - 1);
+    len = MIN(len, TOXIC_MAX_NAME_LENGTH);
     buf[len] = '\0';
     return len;
 }
 
+/* copies data to msg buffer.
+   returns length of msg, which will be no larger than size-1 */
+uint16_t copy_tox_str(char *msg, size_t size, const char *data, uint16_t length)
+{
+    int len = MIN(length, size - 1);
+    memcpy(msg, data, len);
+    msg[len] = '\0';
+    return len;
+}
+
 /* returns index of the first instance of ch in s starting at idx.
    returns length of s if char not found */
 int char_find(int idx, const char *s, char ch)

src/misc_tools.h

@@ -100,6 +100,10 @@ void str_to_lower(char *str);
    Returns nick len on success, -1 on failure */
 int get_nick_truncate(Tox *m, char *buf, int friendnum);
 
+/* copies data to msg buffer.
+   returns length of msg, which will be no larger than size-1 */
+uint16_t copy_tox_str(char *msg, size_t size, const char *data, uint16_t length);
+
 /* returns index of the first instance of ch in s starting at idx.
    returns length of s if char not found */
 int char_find(int idx, const char *s, char ch);

src/prompt.c

@@ -364,8 +364,7 @@ static void prompt_onConnectionChange(ToxWindow *self, Tox *m, int32_t friendnum
     }
 }
 
-static void prompt_onFriendRequest(ToxWindow *self, Tox *m, const char *key, const char *data,
+static void prompt_onFriendRequest(ToxWindow *self, Tox *m, const char *key, const char *data, uint16_t length)
-                                   uint16_t length)
 {
     ChatContext *ctx = self->chatwin;
 

src/toxic.h

@@ -42,7 +42,7 @@
 
 #define UNKNOWN_NAME "Anonymous"
 
-#define MAX_STR_SIZE TOX_MAX_MESSAGE_LENGTH
+#define MAX_STR_SIZE TOX_MAX_MESSAGE_LENGTH    /* must be >= TOX_MAX_MESSAGE_LENGTH */
 #define MAX_CMDNAME_SIZE 64
 #define TOXIC_MAX_NAME_LENGTH 32   /* Must be <= TOX_MAX_NAME_LENGTH */
 #define KEY_IDENT_DIGITS 3    /* number of hex digits to display for the pub-key based identifier */

src/windows.c

@@ -32,6 +32,7 @@
 #include "groupchat.h"
 #include "chat.h"
 #include "line_info.h"
+#include "misc_tools.h"
 
 #include "settings.h"
 extern char *DATA_FILE;
@@ -47,11 +48,14 @@ static int num_active_windows;
 /* CALLBACKS START */
 void on_request(Tox *m, const uint8_t *public_key, const uint8_t *data, uint16_t length, void *userdata)
 {
+    char msg[MAX_STR_SIZE + 1];
+    length = copy_tox_str(msg, sizeof(msg), (const char *) data, length);
+
     int i;
 
     for (i = 0; i < MAX_WINDOWS_NUM; ++i) {
         if (windows[i].onFriendRequest != NULL)
-            windows[i].onFriendRequest(&windows[i], m, (const char *) public_key, (const char *) data, length);
+            windows[i].onFriendRequest(&windows[i], m, (const char *) public_key, msg, length);
     }
 }
 
@@ -80,16 +84,22 @@ void on_typing_change(Tox *m, int32_t friendnumber, uint8_t is_typing, void *use
 
 void on_message(Tox *m, int32_t friendnumber, const uint8_t *string, uint16_t length, void *userdata)
 {
+    char msg[MAX_STR_SIZE + 1];
+    length = copy_tox_str(msg, sizeof(msg), (const char *) string, length);
+
     int i;
 
     for (i = 0; i < MAX_WINDOWS_NUM; ++i) {
         if (windows[i].onMessage != NULL)
-            windows[i].onMessage(&windows[i], m, friendnumber, (const char *) string, length);
+            windows[i].onMessage(&windows[i], m, friendnumber, msg, length);
     }
 }
 
 void on_action(Tox *m, int32_t friendnumber, const uint8_t *string, uint16_t length, void *userdata)
 {
+    char msg[MAX_STR_SIZE + 1];
+    length = copy_tox_str(msg, sizeof(msg), (const char *) string, length);
+
     int i;
 
     for (i = 0; i < MAX_WINDOWS_NUM; ++i) {
@@ -100,11 +110,14 @@ void on_action(Tox *m, int32_t friendnumber, const uint8_t *string, uint16_t len
 
 void on_nickchange(Tox *m, int32_t friendnumber, const uint8_t *string, uint16_t length, void *userdata)
 {
+    char nick[TOXIC_MAX_NAME_LENGTH + 1];
+    length = copy_tox_str(nick, sizeof(nick), (const char *) string, length);
+
     int i;
 
     for (i = 0; i < MAX_WINDOWS_NUM; ++i) {
         if (windows[i].onNickChange != NULL)
-            windows[i].onNickChange(&windows[i], m, friendnumber, (const char *) string, length);
+            windows[i].onNickChange(&windows[i], m, friendnumber, nick, length);
     }
 
     store_data(m, DATA_FILE);
@@ -112,11 +125,14 @@ void on_nickchange(Tox *m, int32_t friendnumber, const uint8_t *string, uint16_t
 
 void on_statusmessagechange(Tox *m, int32_t friendnumber, const uint8_t *string, uint16_t length, void *userdata)
 {
+    char msg[TOX_MAX_STATUSMESSAGE_LENGTH + 1];
+    length = copy_tox_str(msg, sizeof(msg), (const char *) string, length);
+
     int i;
 
     for (i = 0; i < MAX_WINDOWS_NUM; ++i) {
         if (windows[i].onStatusMessageChange != NULL)
-            windows[i].onStatusMessageChange(&windows[i], friendnumber, (const char *) string, length);
+            windows[i].onStatusMessageChange(&windows[i], friendnumber, msg, length);
     }
 }
 
@@ -145,22 +161,28 @@ void on_friendadded(Tox *m, int32_t friendnumber, bool sort)
 void on_groupmessage(Tox *m, int groupnumber, int peernumber, const uint8_t *message, uint16_t length,
                      void *userdata)
 {
+    char msg[MAX_STR_SIZE + 1];
+    length = copy_tox_str(msg, sizeof(msg), (const char *) message, length);
+
     int i;
 
     for (i = 0; i < MAX_WINDOWS_NUM; ++i) {
         if (windows[i].onGroupMessage != NULL)
-            windows[i].onGroupMessage(&windows[i], m, groupnumber, peernumber, (const char *) message, length);
+            windows[i].onGroupMessage(&windows[i], m, groupnumber, peernumber, msg, length);
     }
 }
 
 void on_groupaction(Tox *m, int groupnumber, int peernumber, const uint8_t *action, uint16_t length,
                     void *userdata)
 {
+    char msg[MAX_STR_SIZE + 1];
+    length = copy_tox_str(msg, sizeof(msg), (const char *) action, length);
+
     int i;
 
     for (i = 0; i < MAX_WINDOWS_NUM; ++i) {
         if (windows[i].onGroupAction != NULL)
-            windows[i].onGroupAction(&windows[i], m, groupnumber, peernumber, (const char *) action, length);
+            windows[i].onGroupAction(&windows[i], m, groupnumber, peernumber, msg, length);
     }
 }
 

src/windows.h

@@ -146,7 +146,7 @@ struct ToxWindow {
 
     int active_box; /* For box notify */
     
-    char name[TOXIC_MAX_NAME_LENGTH];
+    char name[TOXIC_MAX_NAME_LENGTH + 1];
     int32_t num;    /* corresponds to friendnumber in chat windows */
     bool active;
     int x;
@@ -168,9 +168,9 @@ struct ToxWindow {
 /* statusbar info holder */
 struct StatusBar {
     WINDOW *topline;
-    char statusmsg[TOX_MAX_STATUSMESSAGE_LENGTH];
+    char statusmsg[TOX_MAX_STATUSMESSAGE_LENGTH + 1];
     uint16_t statusmsg_len;
-    char nick[TOXIC_MAX_NAME_LENGTH];
+    char nick[TOXIC_MAX_NAME_LENGTH + 1];
     int nick_len;
     uint8_t status;
     bool is_online;