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 55a7600 commit 9b3c108
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 @@
92d0601d778487206505dce0bf89242b89b37424dc756808472fef3c22621ade /usr/local/bin/tox-bootstrapd
6468ae67bae96ca47fec38e3aff22809ea8a865df294c8a41cdb681b58a00830 /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;

Check warning on line 153 in toxcore/group_pack.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_pack.c#L153

Added line #L153 was not covered by tests
}

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;

Check warning on line 223 in toxcore/group_pack.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_pack.c#L222-L223

Added lines #L222 - L223 were not covered by tests
}

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;

Check warning on line 239 in toxcore/group_pack.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_pack.c#L238-L239

Added lines #L238 - L239 were not covered by tests
}

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;

Check warning on line 407 in toxcore/group_pack.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_pack.c#L406-L407

Added lines #L406 - L407 were not covered by tests
}

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

0 comments on commit 9b3c108

Please sign in to comment.