From 8d1f7753f679b12a8e57f8b5c4b529bb627ba6c5 Mon Sep 17 00:00:00 2001 From: Nick ODell Date: Fri, 2 Aug 2013 13:21:02 -0600 Subject: [PATCH 1/5] Fix bug where memcpy could overrun buffer --- core/net_crypto.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/net_crypto.c b/core/net_crypto.c index 31fb24be..3233d875 100644 --- a/core/net_crypto.c +++ b/core/net_crypto.c @@ -69,8 +69,8 @@ int encrypt_data(uint8_t *public_key, uint8_t *secret_key, uint8_t *nonce, if (length - crypto_box_BOXZEROBYTES + crypto_box_ZEROBYTES > MAX_DATA_SIZE || length == 0) return -1; - uint8_t temp_plain[MAX_DATA_SIZE + crypto_box_ZEROBYTES - crypto_box_BOXZEROBYTES] = {0}; - uint8_t temp_encrypted[MAX_DATA_SIZE + crypto_box_ZEROBYTES]; + uint8_t temp_plain[MAX_DATA_SIZE + crypto_box_BOXZEROBYTES] = {0}; + uint8_t temp_encrypted[MAX_DATA_SIZE + crypto_box_BOXZEROBYTES]; memcpy(temp_plain + crypto_box_ZEROBYTES, plain, length); /* pad the message with 32 0 bytes. */ @@ -101,8 +101,8 @@ int decrypt_data(uint8_t *public_key, uint8_t *secret_key, uint8_t *nonce, if (length > MAX_DATA_SIZE || length <= crypto_box_BOXZEROBYTES) return -1; - uint8_t temp_plain[MAX_DATA_SIZE - crypto_box_ZEROBYTES + crypto_box_BOXZEROBYTES]; - uint8_t temp_encrypted[MAX_DATA_SIZE + crypto_box_ZEROBYTES] = {0}; + uint8_t temp_plain[MAX_DATA_SIZE + crypto_box_BOXZEROBYTES]; + uint8_t temp_encrypted[MAX_DATA_SIZE + crypto_box_BOXZEROBYTES] = {0}; memcpy(temp_encrypted + crypto_box_BOXZEROBYTES, encrypted, length); /* pad the message with 16 0 bytes. */ From 9c039cfd2a11ba6dc7c3c5bfca376b0f955ff7d3 Mon Sep 17 00:00:00 2001 From: Nick ODell Date: Fri, 2 Aug 2013 16:35:41 -0600 Subject: [PATCH 2/5] Replace ZEROBYTES - BOXZEROBYTES with MACBYTES --- core/net_crypto.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/net_crypto.c b/core/net_crypto.c index 3233d875..3b5b67f4 100644 --- a/core/net_crypto.c +++ b/core/net_crypto.c @@ -66,7 +66,7 @@ static int incoming_connections[MAX_INCOMING]; int encrypt_data(uint8_t *public_key, uint8_t *secret_key, uint8_t *nonce, uint8_t *plain, uint32_t length, uint8_t *encrypted) { - if (length - crypto_box_BOXZEROBYTES + crypto_box_ZEROBYTES > MAX_DATA_SIZE || length == 0) + if (length + crypto_box_MACBYTES > MAX_DATA_SIZE || length == 0) return -1; uint8_t temp_plain[MAX_DATA_SIZE + crypto_box_BOXZEROBYTES] = {0}; @@ -87,7 +87,7 @@ int encrypt_data(uint8_t *public_key, uint8_t *secret_key, uint8_t *nonce, return -1; /* unpad the encrypted message */ - memcpy(encrypted, temp_encrypted + crypto_box_BOXZEROBYTES, length - crypto_box_BOXZEROBYTES + crypto_box_ZEROBYTES); + memcpy(encrypted, temp_encrypted + crypto_box_BOXZEROBYTES, length + crypto_box_MACBYTES); return length - crypto_box_BOXZEROBYTES + crypto_box_ZEROBYTES; } @@ -121,7 +121,7 @@ int decrypt_data(uint8_t *public_key, uint8_t *secret_key, uint8_t *nonce, return -1; /* unpad the plain message */ - memcpy(plain, temp_plain + crypto_box_ZEROBYTES, length - crypto_box_ZEROBYTES + crypto_box_BOXZEROBYTES); + memcpy(plain, temp_plain + crypto_box_ZEROBYTES, length - crypto_box_MACBYTES); return length - crypto_box_ZEROBYTES + crypto_box_BOXZEROBYTES; } From 809ee9af75fefacaf55f69a9449aa14c00c9a502 Mon Sep 17 00:00:00 2001 From: charmlesscoin Date: Fri, 2 Aug 2013 19:51:08 -0400 Subject: [PATCH 3/5] new_lines should use strncpy, and wrote get_id() to declutter code --- testing/nTox.c | 79 +++++++++++++++++++++++--------------------------- 1 file changed, 37 insertions(+), 42 deletions(-) diff --git a/testing/nTox.c b/testing/nTox.c index 81cac0b6..9d44d757 100644 --- a/testing/nTox.c +++ b/testing/nTox.c @@ -44,13 +44,40 @@ int x, y; uint8_t pending_requests[256][CLIENT_ID_SIZE]; uint8_t num_requests = 0; +// 01513F29A0483B3F482478B4A039041E19571249AE1F4E323E4C1C6A446BFF06 +// 01513F29A0483B3F482478B4A039041E19571249AE1F4E323E4C1C6A446BFF06 +// 01513F29A0483B3F482478B4A039041E19571249AE1F4E323E4C1C6A446BFF0 +void get_id(char *data) +{ + char idstring0[200]; + char idstring1[PUB_KEY_BYTES][5]; + char idstring2[PUB_KEY_BYTES][5]; + int i; + for(i = 0; i < PUB_KEY_BYTES; i++) + { + if (self_public_key[i] < (PUB_KEY_BYTES / 2)) + strcpy(idstring1[i],"0"); + else + strcpy(idstring1[i], ""); + sprintf(idstring2[i], "%hhX",self_public_key[i]); + } + strcpy(idstring0,"[i] ID: "); + int j; + for (j = 0; j < PUB_KEY_BYTES; j++) { + strcat(idstring0,idstring1[j]); + strcat(idstring0,idstring2[j]); + } + + memcpy(data, idstring0, strlen(idstring0)); +} + void new_lines(char *line) { - int i; + int i = 0; for (i = HISTORY-1; i > 0; i--) - strcpy(lines[i], lines[i-1]); + strncpy(lines[i], lines[i-1], STRING_LENGTH - 1); - strcpy(lines[0], line); + strncpy(lines[0], line, STRING_LENGTH - 1); do_refresh(); } @@ -216,26 +243,9 @@ void line_eval(char lines[HISTORY][STRING_LENGTH], char *line) new_lines("[i] /l list (list friends), /h for help, /i for info, /n nick (to change nickname), /q (to quit)"); } else if (inpt_command == 'i') { //info - char idstring0[200]; - char idstring1[PUB_KEY_BYTES][5]; - char idstring2[PUB_KEY_BYTES][5]; - int i; - for (i = 0; i < PUB_KEY_BYTES; i++) - { - if (self_public_key[i] < (PUB_KEY_BYTES/2)) - strcpy(idstring1[i],"0"); - else - strcpy(idstring1[i], ""); - sprintf(idstring2[i], "%hhX", self_public_key[i]); - } - // - strcpy(idstring0,"[i] ID: "); - int j; - for (j = 0; j < PUB_KEY_BYTES; j++) { - strcat(idstring0,idstring1[j]); - strcat(idstring0,idstring2[j]); - } - new_lines(idstring0); + char idstring[200]; + get_id(idstring); + new_lines(idstring); } else if (inpt_command == 'q') { //exit @@ -396,29 +406,14 @@ int main(int argc, char *argv[]) m_callback_friendmessage(print_message); m_callback_namechange(print_nickchange); m_callback_userstatus(print_statuschange); - char idstring0[200]; - char idstring1[PUB_KEY_BYTES][5]; - char idstring2[PUB_KEY_BYTES][5]; - int i; - for(i = 0; i < PUB_KEY_BYTES; i++) - { - if (self_public_key[i] < (PUB_KEY_BYTES / 2)) - strcpy(idstring1[i],"0"); - else - strcpy(idstring1[i], ""); - sprintf(idstring2[i], "%hhX",self_public_key[i]); - } - strcpy(idstring0,"[i] your ID: "); - int j; - for (j = 0; j < PUB_KEY_BYTES; j++) { - strcat(idstring0,idstring1[j]); - strcat(idstring0,idstring2[j]); - } + + char idstring[200]; + get_id(idstring); initscr(); noecho(); raw(); getmaxyx(stdscr, y, x); - new_lines(idstring0); + new_lines(idstring); new_lines(help); strcpy(line, ""); IP_Port bootstrap_ip_port; From f906fd628fd7cc8531d9f9756880c198e5a10a68 Mon Sep 17 00:00:00 2001 From: charmlesscoin Date: Fri, 2 Aug 2013 19:54:57 -0400 Subject: [PATCH 4/5] cleaned up my changes --- testing/nTox.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/testing/nTox.c b/testing/nTox.c index 9d44d757..24d40ead 100644 --- a/testing/nTox.c +++ b/testing/nTox.c @@ -44,15 +44,12 @@ int x, y; uint8_t pending_requests[256][CLIENT_ID_SIZE]; uint8_t num_requests = 0; -// 01513F29A0483B3F482478B4A039041E19571249AE1F4E323E4C1C6A446BFF06 -// 01513F29A0483B3F482478B4A039041E19571249AE1F4E323E4C1C6A446BFF06 -// 01513F29A0483B3F482478B4A039041E19571249AE1F4E323E4C1C6A446BFF0 void get_id(char *data) { char idstring0[200]; char idstring1[PUB_KEY_BYTES][5]; char idstring2[PUB_KEY_BYTES][5]; - int i; + int i = 0; for(i = 0; i < PUB_KEY_BYTES; i++) { if (self_public_key[i] < (PUB_KEY_BYTES / 2)) @@ -62,7 +59,7 @@ void get_id(char *data) sprintf(idstring2[i], "%hhX",self_public_key[i]); } strcpy(idstring0,"[i] ID: "); - int j; + int j = 0; for (j = 0; j < PUB_KEY_BYTES; j++) { strcat(idstring0,idstring1[j]); strcat(idstring0,idstring2[j]); From 9e5800ab10c3f006decb6a02f151db18e3bbb763 Mon Sep 17 00:00:00 2001 From: irungentoo Date: Fri, 2 Aug 2013 20:21:49 -0400 Subject: [PATCH 5/5] Fixed regression. --- core/Lossless_UDP.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/core/Lossless_UDP.c b/core/Lossless_UDP.c index 4affc38f..33b8eb19 100644 --- a/core/Lossless_UDP.c +++ b/core/Lossless_UDP.c @@ -202,15 +202,16 @@ int new_connection(IP_Port ip_port) for (i = 0; i < MAX_CONNECTIONS; ++i) { if(connections[i].status == 0) { memset(&connections[i], 0, sizeof(Connection)); + uint32_t handshake_id1 = handshake_id(ip_port); connections[i] = (Connection) { .ip_port = ip_port, .status = 1, .inbound = 0, - .handshake_id1 = handshake_id(ip_port), - .sent_packetnum = connections[i].handshake_id1, - .sendbuff_packetnum = connections[i].handshake_id1, - .successful_sent = connections[i].handshake_id1, + .handshake_id1 = handshake_id1, + .sent_packetnum = handshake_id1, + .sendbuff_packetnum = handshake_id1, + .successful_sent = handshake_id1, .SYNC_rate = SYNC_RATE, .data_rate = DATA_SYNC_RATE, .last_recvSYNC = current_time(), @@ -254,6 +255,7 @@ int new_inconnection(IP_Port ip_port) for (i = 0; i < MAX_CONNECTIONS; ++i) { if (connections[i].status == 0) { memset(&connections[i], 0, sizeof(Connection)); + uint64_t timeout = CONNEXION_TIMEOUT + rand() % CONNEXION_TIMEOUT; connections[i] = (Connection){ .ip_port = ip_port, @@ -266,10 +268,10 @@ int new_inconnection(IP_Port ip_port) .send_counter = 127, /* add randomness to timeout to prevent connections getting stuck in a loop. */ - .timeout = CONNEXION_TIMEOUT + rand() % CONNEXION_TIMEOUT, + .timeout = timeout, /* if this connection isn't handled within the timeout kill it. */ - .killat = current_time() + 1000000UL*connections[i].timeout + .killat = current_time() + 1000000UL*timeout }; ++connections_number; return i;