Skip to content

Commit

Permalink
refactor: Add a bin_unpack_bin_max for max-length arrays.
Browse files Browse the repository at this point in the history
These are statically allocated (e.g. `uint8_t[1024]`) arrays with
variable length data inside them. Examples are group topics and
nicknames.
  • Loading branch information
iphydf committed Nov 11, 2023
1 parent 82276ef commit d99ebed
Show file tree
Hide file tree
Showing 22 changed files with 79 additions and 39 deletions.
4 changes: 4 additions & 0 deletions toxcore/Messenger.c
Original file line number Diff line number Diff line change
Expand Up @@ -3219,9 +3219,13 @@ static State_Load_Status groups_load(Messenger *m, const uint8_t *data, uint32_t

if (group_number < 0) {
LOGGER_WARNING(m->log, "Failed to load group %u", i);
// Can't recover trivially. We may need to skip over some data here.
break;
}
}

LOGGER_DEBUG(m->log, "Successfully loaded %u groups", gc_count_groups(m->group_handler));

bin_unpack_free(bu);

return STATE_LOAD_STATUS_CONTINUE;
Expand Down
26 changes: 23 additions & 3 deletions toxcore/bin_unpack.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,14 @@ bool bin_unpack_array(Bin_Unpack *bu, uint32_t *size)
return cmp_read_array(&bu->ctx, size) && *size <= bu->bytes_size;
}

bool bin_unpack_array_fixed(Bin_Unpack *bu, uint32_t required_size)
bool bin_unpack_array_fixed(Bin_Unpack *bu, uint32_t required_size, uint32_t *actual_size)
{
uint32_t size;
return cmp_read_array(&bu->ctx, &size) && size == required_size;
uint32_t size = 0;
const bool success = cmp_read_array(&bu->ctx, &size) && size == required_size;
if (actual_size != nullptr) {
*actual_size = size;
}
return success;
}

bool bin_unpack_bool(Bin_Unpack *bu, bool *val)
Expand Down Expand Up @@ -128,6 +132,22 @@ bool bin_unpack_bin(Bin_Unpack *bu, uint8_t **data_ptr, uint32_t *data_length_pt
return true;
}

bool bin_unpack_bin_max(Bin_Unpack *bu, uint8_t *data, uint16_t *data_length_ptr, uint16_t max_data_length)
{
uint32_t bin_size;
if (!bin_unpack_bin_size(bu, &bin_size) || bin_size > max_data_length) {
return false;
}

*data_length_ptr = bin_size;

if (!bin_unpack_bin_b(bu, data, bin_size)) {
return false;
}

return true;
}

bool bin_unpack_bin_fixed(Bin_Unpack *bu, uint8_t *data, uint32_t data_length)
{
uint32_t bin_size;
Expand Down
12 changes: 9 additions & 3 deletions toxcore/bin_unpack.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ non_null() bool bin_unpack_array(Bin_Unpack *bu, uint32_t *size);
*
* @retval false if the packed array size is not exactly the required size.
*/
non_null() bool bin_unpack_array_fixed(Bin_Unpack *bu, uint32_t required_size);
non_null() bool bin_unpack_array_fixed(Bin_Unpack *bu, uint32_t required_size, uint32_t *actual_size);

/** @brief Unpack a MessagePack bool. */
non_null() bool bin_unpack_bool(Bin_Unpack *bu, bool *val);
Expand All @@ -71,10 +71,16 @@ non_null() bool bin_unpack_nil(Bin_Unpack *bu);
* large allocation unless the input array was already that large.
*/
non_null() bool bin_unpack_bin(Bin_Unpack *bu, uint8_t **data_ptr, uint32_t *data_length_ptr);
/** @brief Unpack a variable size MessagePack bin into a fixed size byte array.
*
* Stores unpacked data into `data` with its length stored in `data_length_ptr`. This function does
* not allocate memory and requires that `max_data_length` is less than or equal to `sizeof(arr)`
* when `arr` is passed as `data` pointer.
*/
non_null() bool bin_unpack_bin_max(Bin_Unpack *bu, uint8_t *data, uint16_t *data_length_ptr, uint16_t max_data_length);
/** @brief Unpack a MessagePack bin of a fixed length into a pre-allocated byte array.
*
* Unlike the function above, this function does not allocate any memory, but requires the size to
* be known up front.
* Similar to the function above, but doesn't output the data length.
*/
non_null() bool bin_unpack_bin_fixed(Bin_Unpack *bu, uint8_t *data, uint32_t data_length);

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/conference_invite.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ static bool tox_event_conference_invite_unpack(
Tox_Event_Conference_Invite *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 3)) {
if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/conference_message.c
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ static bool tox_event_conference_message_unpack(
Tox_Event_Conference_Message *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 4)) {
if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/conference_peer_name.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ static bool tox_event_conference_peer_name_unpack(
Tox_Event_Conference_Peer_Name *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 3)) {
if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/conference_title.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ static bool tox_event_conference_title_unpack(
Tox_Event_Conference_Title *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 3)) {
if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/file_chunk_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ static bool tox_event_file_chunk_request_unpack(
Tox_Event_File_Chunk_Request *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 4)) {
if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/file_recv.c
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ static bool tox_event_file_recv_unpack(
Tox_Event_File_Recv *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 5)) {
if (!bin_unpack_array_fixed(bu, 5, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/file_recv_chunk.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ static bool tox_event_file_recv_chunk_unpack(
Tox_Event_File_Recv_Chunk *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 4)) {
if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/file_recv_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ static bool tox_event_file_recv_control_unpack(
Tox_Event_File_Recv_Control *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 3)) {
if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_connection_status.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ static bool tox_event_friend_connection_status_unpack(
Tox_Event_Friend_Connection_Status *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_lossless_packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ static bool tox_event_friend_lossless_packet_unpack(
Tox_Event_Friend_Lossless_Packet *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_lossy_packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ static bool tox_event_friend_lossy_packet_unpack(
Tox_Event_Friend_Lossy_Packet *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_message.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ static bool tox_event_friend_message_unpack(
Tox_Event_Friend_Message *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 3)) {
if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_name.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ static bool tox_event_friend_name_unpack(
Tox_Event_Friend_Name *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_read_receipt.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ static bool tox_event_friend_read_receipt_unpack(
Tox_Event_Friend_Read_Receipt *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ static bool tox_event_friend_request_unpack(
Tox_Event_Friend_Request *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_status.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ static bool tox_event_friend_status_unpack(
Tox_Event_Friend_Status *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_status_message.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ static bool tox_event_friend_status_message_unpack(
Tox_Event_Friend_Status_Message *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/events/friend_typing.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ static bool tox_event_friend_typing_unpack(
Tox_Event_Friend_Typing *event, Bin_Unpack *bu)
{
assert(event != nullptr);
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
return false;
}

Expand Down
40 changes: 25 additions & 15 deletions toxcore/group_pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
non_null()
static bool load_unpack_state_values(GC_Chat *chat, Bin_Unpack *bu)
{
if (!bin_unpack_array_fixed(bu, 8)) {
if (!bin_unpack_array_fixed(bu, 8, nullptr)) {
LOGGER_ERROR(chat->log, "Group state values array malformed");
return false;
}
Expand Down Expand Up @@ -58,15 +58,23 @@ static bool load_unpack_state_values(GC_Chat *chat, Bin_Unpack *bu)
non_null()
static bool load_unpack_state_bin(GC_Chat *chat, Bin_Unpack *bu)
{
if (!bin_unpack_array_fixed(bu, 5)) {
if (!bin_unpack_array_fixed(bu, 5, nullptr)) {
LOGGER_ERROR(chat->log, "Group state binary array malformed");
return false;
}

if (!(bin_unpack_bin_fixed(bu, chat->shared_state_sig, SIGNATURE_SIZE)
&& bin_unpack_bin_fixed(bu, chat->shared_state.founder_public_key, EXT_PUBLIC_KEY_SIZE)
&& bin_unpack_bin_fixed(bu, chat->shared_state.group_name, chat->shared_state.group_name_len)
&& bin_unpack_bin_fixed(bu, chat->shared_state.password, chat->shared_state.password_length)
if (!bin_unpack_bin_fixed(bu, chat->shared_state_sig, SIGNATURE_SIZE)) {
LOGGER_ERROR(chat->log, "Failed to unpack shared state signature");
return false;
}

if (!bin_unpack_bin_fixed(bu, chat->shared_state.founder_public_key, EXT_PUBLIC_KEY_SIZE)) {
LOGGER_ERROR(chat->log, "Failed to unpack founder public key");
return false;
}

if (!(bin_unpack_bin_max(bu, chat->shared_state.group_name, &chat->shared_state.group_name_len, sizeof(chat->shared_state.group_name))
&& bin_unpack_bin_max(bu, chat->shared_state.password, &chat->shared_state.password_length, sizeof(chat->shared_state.password))
&& bin_unpack_bin_fixed(bu, chat->shared_state.mod_list_hash, MOD_MODERATION_HASH_SIZE))) {
LOGGER_ERROR(chat->log, "Failed to unpack state binary data");
return false;
Expand All @@ -78,15 +86,15 @@ static bool load_unpack_state_bin(GC_Chat *chat, Bin_Unpack *bu)
non_null()
static bool load_unpack_topic_info(GC_Chat *chat, Bin_Unpack *bu)
{
if (!bin_unpack_array_fixed(bu, 6)) {
if (!bin_unpack_array_fixed(bu, 6, nullptr)) {
LOGGER_ERROR(chat->log, "Group topic array malformed");
return false;
}

if (!(bin_unpack_u32(bu, &chat->topic_info.version)
&& bin_unpack_u16(bu, &chat->topic_info.length)
&& bin_unpack_u16(bu, &chat->topic_info.checksum)
&& bin_unpack_bin_fixed(bu, chat->topic_info.topic, chat->topic_info.length)
&& bin_unpack_bin_max(bu, chat->topic_info.topic, &chat->topic_info.length, sizeof(chat->topic_info.topic))
&& bin_unpack_bin_fixed(bu, chat->topic_info.public_sig_key, SIG_PUBLIC_KEY_SIZE)
&& bin_unpack_bin_fixed(bu, chat->topic_sig, SIGNATURE_SIZE))) {
LOGGER_ERROR(chat->log, "Failed to unpack topic info");
Expand All @@ -99,8 +107,9 @@ static bool load_unpack_topic_info(GC_Chat *chat, Bin_Unpack *bu)
non_null()
static bool load_unpack_mod_list(GC_Chat *chat, Bin_Unpack *bu)
{
if (!bin_unpack_array_fixed(bu, 2)) {
LOGGER_ERROR(chat->log, "Group mod list array malformed");
uint32_t actual_size = 0;
if (!bin_unpack_array_fixed(bu, 2, &actual_size)) {
LOGGER_ERROR(chat->log, "Group mod list array malformed: %d != 2", actual_size);
return false;
}

Expand Down Expand Up @@ -148,7 +157,7 @@ static bool load_unpack_mod_list(GC_Chat *chat, Bin_Unpack *bu)
non_null()
static bool load_unpack_keys(GC_Chat *chat, Bin_Unpack *bu)
{
if (!bin_unpack_array_fixed(bu, 4)) {
if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
LOGGER_ERROR(chat->log, "Group keys array malformed");
return false;
}
Expand All @@ -167,7 +176,7 @@ static bool load_unpack_keys(GC_Chat *chat, Bin_Unpack *bu)
non_null()
static bool load_unpack_self_info(GC_Chat *chat, Bin_Unpack *bu)
{
if (!bin_unpack_array_fixed(bu, 4)) {
if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
LOGGER_ERROR(chat->log, "Group self info array malformed");
return false;
}
Expand Down Expand Up @@ -214,7 +223,7 @@ static bool load_unpack_self_info(GC_Chat *chat, Bin_Unpack *bu)
non_null()
static bool load_unpack_saved_peers(GC_Chat *chat, Bin_Unpack *bu)
{
if (!bin_unpack_array_fixed(bu, 2)) {
if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
LOGGER_ERROR(chat->log, "Group saved peers array malformed");
return false;
}
Expand Down Expand Up @@ -256,8 +265,9 @@ static bool load_unpack_saved_peers(GC_Chat *chat, Bin_Unpack *bu)

bool gc_load_unpack_group(GC_Chat *chat, Bin_Unpack *bu)
{
if (!bin_unpack_array_fixed(bu, 7)) {
LOGGER_ERROR(chat->log, "Group info array malformed");
uint32_t actual_size;
if (!bin_unpack_array_fixed(bu, 7, &actual_size)) {
LOGGER_ERROR(chat->log, "Group info array malformed: %d != 7", actual_size);
return false;
}

Expand Down

0 comments on commit d99ebed

Please sign in to comment.