Skip to content

Commit

Permalink
Make group saving/loading more forgiving with data errors
Browse files Browse the repository at this point in the history
Rather than aborting the process on invalid group save data we
either try to continue if possible, or abort the saving/loading
instead of the entire process
  • Loading branch information
JFreegman committed Dec 14, 2023
1 parent 6645343 commit 2a6262c
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3c31e0cef0463b0a37bb21c5322c1a219314404b2a93193f4fba49f901a9dfb6 /usr/local/bin/tox-bootstrapd
13daeb19f65bac2686f11fbd8702cdc5fdbb365826659aa736a4a0bdb16b3487 /usr/local/bin/tox-bootstrapd
3 changes: 2 additions & 1 deletion toxcore/group_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,8 @@ int unpack_gc_saved_peers(GC_Chat *chat, const uint8_t *data, uint16_t length);

/** @brief Packs all valid entries from saved peerlist into `data`.
*
* If `processed` is non-null it will be set to the length of the packed data.
* If `processed` is non-null it will be set to the length of the packed data
* on success, and will be untouched on error.
*
* Return the number of packed saved peers on success.
* Return -1 if buffer is too small.
Expand Down
20 changes: 14 additions & 6 deletions toxcore/group_pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#include "group_pack.h"

#include <assert.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
Expand Down Expand Up @@ -151,7 +150,7 @@ static bool load_unpack_mod_list(GC_Chat *chat, Bin_Unpack *bu)

if (chat->moderation.num_mods > MOD_MAX_NUM_MODERATORS) {
LOGGER_ERROR(chat->log, "moderation count %u exceeds maximum %u", chat->moderation.num_mods, MOD_MAX_NUM_MODERATORS);
return false;
chat->moderation.num_mods = MOD_MAX_NUM_MODERATORS;
}

uint8_t *packed_mod_list = (uint8_t *)malloc(chat->moderation.num_mods * MOD_LIST_ENTRY_SIZE);
Expand Down Expand Up @@ -219,7 +218,10 @@ static bool load_unpack_self_info(GC_Chat *chat, Bin_Unpack *bu)
return false;
}

assert(self_nick_len <= MAX_GC_NICK_SIZE);
if (self_nick_len > MAX_GC_NICK_SIZE) {
LOGGER_ERROR(chat->log, "self_nick too big (%u bytes), truncating to %d", self_nick_len, MAX_GC_NICK_SIZE);
self_nick_len = MAX_GC_NICK_SIZE;
}

if (!bin_unpack_bin_fixed(bu, self_nick, self_nick_len)) {
LOGGER_ERROR(chat->log, "Failed to unpack self nick bytes");
Expand All @@ -232,7 +234,10 @@ static bool load_unpack_self_info(GC_Chat *chat, Bin_Unpack *bu)
return false;
}

assert(chat->numpeers > 0);
if (chat->numpeers == 0) {
LOGGER_ERROR(chat->log, "Failed to unpack self: numpeers should be > 0");
return false;
}

GC_Peer *self = &chat->group[0];

Expand Down Expand Up @@ -395,9 +400,12 @@ static void save_pack_self_info(const GC_Chat *chat, Bin_Pack *bp)
{
bin_pack_array(bp, 4);

const GC_Peer *self = &chat->group[0];
GC_Peer *self = &chat->group[0];

assert(self->nick_length <= MAX_GC_NICK_SIZE);
if (self->nick_length > MAX_GC_NICK_SIZE) {
LOGGER_ERROR(chat->log, "self_nick is too big (%u). Truncating to %d", self->nick_length, MAX_GC_NICK_SIZE);
self->nick_length = MAX_GC_NICK_SIZE;
}

bin_pack_u16(bp, self->nick_length); // 1
bin_pack_u08(bp, (uint8_t)self->role); // 2
Expand Down

0 comments on commit 2a6262c

Please sign in to comment.