From ec659491b265db747e67ef8a166d224c0453397d Mon Sep 17 00:00:00 2001 From: irungentoo Date: Sun, 15 Mar 2015 18:35:22 -0400 Subject: Avatar hash is now the filename of the file transfer instead of the first 32 bytes. Enforce length of filename in core when transfer is an avatar type transfer to make things more safe. --- toxcore/Messenger.c | 18 +++++++++++++----- toxcore/Messenger.h | 8 +++++++- toxcore/tox.c | 2 +- toxcore/tox.h | 12 +++++++----- 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 83aaf19b..911c92da 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -1051,7 +1051,7 @@ static int file_sendrequest(const Messenger *m, int32_t friendnumber, uint8_t fi * Maximum filename length is 255 bytes. * return file number on success * return -1 if friend not found. - * return -2 if filename too big. + * return -2 if filename length invalid. * return -3 if no more file sending slots left. * return -4 if could not send packet (friend offline). * @@ -1065,6 +1065,9 @@ long int new_filesender(const Messenger *m, int32_t friendnumber, uint32_t file_ if (filename_length > MAX_FILENAME_LENGTH) return -2; + if (file_type == FILEKIND_AVATAR && filename_length != crypto_hash_sha256_BYTES) + return -2; + uint32_t i; for (i = 0; i < MAX_CONCURRENT_FILE_PIPES; ++i) { @@ -1937,9 +1940,14 @@ static int handle_packet(void *object, int i, uint8_t *temp, uint16_t len) uint8_t filenumber = data[0]; uint64_t filesize; uint32_t file_type; + uint16_t filename_length = data_length - head_length; memcpy(&file_type, data + 1, sizeof(file_type)); file_type = ntohl(file_type); + /* Check if the name is the right size if file is avatar. */ + if (file_type == FILEKIND_AVATAR && filename_length != crypto_hash_sha256_BYTES) + break; + memcpy(&filesize, data + 1 + sizeof(uint32_t), sizeof(filesize)); net_to_host((uint8_t *) &filesize, sizeof(filesize)); m->friendlist[i].file_receiving[filenumber].status = FILESTATUS_NOT_ACCEPTED; @@ -1948,16 +1956,16 @@ static int handle_packet(void *object, int i, uint8_t *temp, uint16_t len) m->friendlist[i].file_receiving[filenumber].paused = FILE_PAUSE_NOT; /* Force NULL terminate file name. */ - uint8_t filename_terminated[data_length - head_length + 1]; - memcpy(filename_terminated, data + head_length, data_length - head_length); - filename_terminated[data_length - head_length] = 0; + uint8_t filename_terminated[filename_length + 1]; + memcpy(filename_terminated, data + head_length, filename_length); + filename_terminated[filename_length] = 0; uint32_t real_filenumber = filenumber; real_filenumber += 1; real_filenumber <<= 16; if (m->file_sendrequest) - (*m->file_sendrequest)(m, i, real_filenumber, file_type, filesize, filename_terminated, data_length - head_length, + (*m->file_sendrequest)(m, i, real_filenumber, file_type, filesize, filename_terminated, filename_length, m->file_sendrequest_userdata); break; diff --git a/toxcore/Messenger.h b/toxcore/Messenger.h index d4cfa431..716ac851 100644 --- a/toxcore/Messenger.h +++ b/toxcore/Messenger.h @@ -159,6 +159,12 @@ enum { FILECONTROL_RESUME_BROKEN }; +enum { + FILEKIND_DATA, + FILEKIND_AVATAR +}; + + typedef struct Messenger Messenger; typedef struct { @@ -608,7 +614,7 @@ void callback_file_reqchunk(Messenger *m, void (*function)(Messenger *m, uint32_ * Maximum filename length is 255 bytes. * return file number on success * return -1 if friend not found. - * return -2 if filename too big. + * return -2 if filename length invalid. * return -3 if no more file sending slots left. * return -4 if could not send packet (friend offline). * diff --git a/toxcore/tox.c b/toxcore/tox.c index 5493d1d5..51aa85ad 100644 --- a/toxcore/tox.c +++ b/toxcore/tox.c @@ -915,7 +915,7 @@ uint32_t tox_file_send(Tox *tox, uint32_t friend_number, uint32_t kind, uint64_t return UINT32_MAX; case -2: - SET_ERROR_PARAMETER(error, TOX_ERR_FILE_SEND_NAME_TOO_LONG); + SET_ERROR_PARAMETER(error, TOX_ERR_FILE_SEND_NAME_INVALID_LENGTH); return UINT32_MAX; case -3: diff --git a/toxcore/tox.h b/toxcore/tox.h index 286d323a..1457a70b 100644 --- a/toxcore/tox.h +++ b/toxcore/tox.h @@ -1396,7 +1396,8 @@ enum TOX_FILE_KIND { */ TOX_FILE_KIND_DATA, /** - * Avatar data. This consists of tox_hash(image) + image. + * Avatar filename. This consists of tox_hash(image). + * Avatar data. This consists of the image data. * * Avatars can be sent at any time the client wishes. Generally, a client will * send the avatar to a friend when that friend comes online, and to all @@ -1406,8 +1407,8 @@ enum TOX_FILE_KIND { * * Clients who receive avatar send requests can reject it (by sending * TOX_FILE_CONTROL_CANCEL before any other controls), or accept it (by - * sending TOX_FILE_CONTROL_RESUME). The first chunk will contain the hash in - * its first TOX_HASH_LENGTH bytes. A client can compare this hash with a + * sending TOX_FILE_CONTROL_RESUME). The filename of length TOX_HASH_LENGTH bytes + * will contain the hash. A client can compare this hash with a * saved hash and send TOX_FILE_CONTROL_CANCEL to terminate the avatar * transfer if it matches. */ @@ -1551,9 +1552,10 @@ typedef enum TOX_ERR_FILE_SEND { */ TOX_ERR_FILE_SEND_NAME_EMPTY, /** - * Filename length exceeded 255 bytes. + * Filename length exceeded 255 bytes or if kind was equal to TOX_FILE_KIND_AVATAR + * the length was not TOX_HASH_LENGTH. */ - TOX_ERR_FILE_SEND_NAME_TOO_LONG, + TOX_ERR_FILE_SEND_NAME_INVALID_LENGTH, /** * Too many ongoing transfers. The maximum number of concurrent file transfers * is 256 per friend per direction (sending and receiving). -- cgit v1.2.3