From 23cf9686cbe00b9531b4dfe5c21e608102624437 Mon Sep 17 00:00:00 2001 From: Jfreegman Date: Fri, 3 Oct 2014 17:53:50 -0400 Subject: [PATCH] safer string handling --- src/chat.c | 17 ++++++----------- src/dns.c | 30 +++++++++++++++--------------- src/friendlist.c | 16 ++++++---------- src/friendlist.h | 6 +++--- src/groupchat.c | 18 +++++++----------- src/line_info.h | 4 ++-- src/misc_tools.c | 12 +++++++++++- src/misc_tools.h | 4 ++++ src/prompt.c | 3 +-- src/toxic.h | 2 +- src/windows.c | 34 ++++++++++++++++++++++++++++------ src/windows.h | 6 +++--- 12 files changed, 87 insertions(+), 65 deletions(-) diff --git a/src/chat.c b/src/chat.c index f77c0e5..36c4aeb 100644 --- a/src/chat.c +++ b/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]; - strcpy(tmpname, nick); - int n_len = MIN(len, TOXIC_MAX_NAME_LENGTH - 1); - tmpname[n_len] = '\0'; + snprintf(statusbar->nick, sizeof(statusbar->nick), "%s", nick); + len = strlen(statusbar->nick); + statusbar->nick_len = len; - snprintf(statusbar->nick, sizeof(statusbar->nick), "%s", tmpname); - chat_set_window_name(self, tmpname, n_len); + chat_set_window_name(self, statusbar->nick, 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; diff --git a/src/dns.c b/src/dns.c index 675d27f..fde57d9 100644 --- a/src/dns.c +++ b/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; } diff --git a/src/friendlist.c b/src/friendlist.c index 05a17fa..1308ba0 100644 --- a/src/friendlist.c +++ b/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]; - strcpy(tempname, nick); - 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; + snprintf(Friends.list[num].name, sizeof(Friends.list[num].name), "%s", nick); + Friends.list[num].namelength = strlen(Friends.list[num].name); /* 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) diff --git a/src/friendlist.h b/src/friendlist.h index 06ef781..c8b3ba3 100644 --- a/src/friendlist.h +++ b/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; diff --git a/src/groupchat.c b/src/groupchat.c index 523f03d..f4b897a 100644 --- a/src/groupchat.c +++ b/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); - write_to_log(msg_cpy, nick, ctx->log, false); + line_info_add(self, timefrmt, nick, NULL, IN_MSG, 0, nick_clr, "%s", msg); + 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); diff --git a/src/line_info.h b/src/line_info.h index 454e8fa..f2590ff 100644 --- a/src/line_info.h +++ b/src/line_info.h @@ -46,8 +46,8 @@ enum { struct line_info { char timestr[TIME_STR_SIZE]; - char name1[TOXIC_MAX_NAME_LENGTH]; - char name2[TOXIC_MAX_NAME_LENGTH]; + char name1[TOXIC_MAX_NAME_LENGTH + 1]; + char name2[TOXIC_MAX_NAME_LENGTH + 1]; char msg[MAX_LINE_INFO_MSG_SIZE]; uint64_t timestamp; uint8_t type; diff --git a/src/misc_tools.c b/src/misc_tools.c index d88c53d..52b0337 100644 --- a/src/misc_tools.c +++ b/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) diff --git a/src/misc_tools.h b/src/misc_tools.h index 631c391..38a6f79 100644 --- a/src/misc_tools.h +++ b/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); diff --git a/src/prompt.c b/src/prompt.c index 9a7fd58..1f336b7 100644 --- a/src/prompt.c +++ b/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, - uint16_t length) +static void prompt_onFriendRequest(ToxWindow *self, Tox *m, const char *key, const char *data, uint16_t length) { ChatContext *ctx = self->chatwin; diff --git a/src/toxic.h b/src/toxic.h index 6f1898f..836991d 100644 --- a/src/toxic.h +++ b/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 */ diff --git a/src/windows.c b/src/windows.c index bf29fd0..b667dbf 100644 --- a/src/windows.c +++ b/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); } } diff --git a/src/windows.h b/src/windows.h index c99f6d7..be3950e 100644 --- a/src/windows.h +++ b/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;