Skip to content

Commit

Permalink
fix: Enable debug flag for ubsan.
Browse files Browse the repository at this point in the history
Otherwise the nullability annotations are compiled out in attributes.h.
  • Loading branch information
iphydf committed Dec 9, 2023
1 parent 4d1db21 commit 3983369
Show file tree
Hide file tree
Showing 12 changed files with 150 additions and 20 deletions.
1 change: 1 addition & 0 deletions .circleci/cmake-ubsan
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ CACHEDIR="$HOME/cache"

. ".github/scripts/flags-$CC.sh"
add_flag -Werror
add_flag -D_DEBUG
add_flag -fdiagnostics-color=always
add_flag -fno-omit-frame-pointer
add_flag -fno-sanitize-recover=all
Expand Down
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 @@
949fc78e9b46da0ea6929c64263767b563012cc1ea0205fb89466422477326ab /usr/local/bin/tox-bootstrapd
3c31e0cef0463b0a37bb21c5322c1a219314404b2a93193f4fba49f901a9dfb6 /usr/local/bin/tox-bootstrapd
5 changes: 5 additions & 0 deletions toxcore/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ cc_test(

cc_fuzz_test(
name = "DHT_fuzz_test",
size = "small",
srcs = ["DHT_fuzz_test.cc"],
corpus = ["//tools/toktok-fuzzer/corpus:DHT_fuzz_test"],
deps = [
Expand Down Expand Up @@ -408,6 +409,7 @@ cc_library(

cc_fuzz_test(
name = "forwarding_fuzz_test",
size = "small",
srcs = ["forwarding_fuzz_test.cc"],
#corpus = ["//tools/toktok-fuzzer/corpus:forwarding_fuzz_test"],
deps = [
Expand Down Expand Up @@ -606,6 +608,7 @@ cc_library(

cc_fuzz_test(
name = "group_announce_fuzz_test",
size = "small",
srcs = ["group_announce_fuzz_test.cc"],
#corpus = ["//tools/toktok-fuzzer/corpus:group_announce_fuzz_test"],
deps = [
Expand Down Expand Up @@ -709,6 +712,7 @@ cc_test(

cc_fuzz_test(
name = "group_moderation_fuzz_test",
size = "small",
srcs = ["group_moderation_fuzz_test.cc"],
corpus = ["//tools/toktok-fuzzer/corpus:group_moderation_fuzz_test"],
deps = [
Expand Down Expand Up @@ -865,6 +869,7 @@ cc_test(

cc_fuzz_test(
name = "tox_events_fuzz_test",
size = "small",
srcs = ["tox_events_fuzz_test.cc"],
corpus = ["//tools/toktok-fuzzer/corpus:tox_events_fuzz_test"],
deps = [
Expand Down
26 changes: 21 additions & 5 deletions toxcore/Messenger.c
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,20 @@ int m_set_statusmessage(Messenger *m, const uint8_t *status, uint16_t length)
return 0;
}

static Userstatus userstatus_from_int(uint8_t status)
{
switch (status) {
case 0:
return USERSTATUS_NONE;
case 1:
return USERSTATUS_AWAY;
case 2:
return USERSTATUS_BUSY;
default:
return USERSTATUS_INVALID;
}
}

int m_set_userstatus(Messenger *m, uint8_t status)
{
if (status >= USERSTATUS_INVALID) {
Expand All @@ -767,7 +781,7 @@ int m_set_userstatus(Messenger *m, uint8_t status)
return 0;
}

m->userstatus = (Userstatus)status;
m->userstatus = userstatus_from_int(status);

for (uint32_t i = 0; i < m->numfriends; ++i) {
m->friendlist[i].userstatus_sent = false;
Expand Down Expand Up @@ -923,7 +937,7 @@ static int set_friend_statusmessage(const Messenger *m, int32_t friendnumber, co
non_null()
static void set_friend_userstatus(const Messenger *m, int32_t friendnumber, uint8_t status)
{
m->friendlist[friendnumber].userstatus = (Userstatus)status;
m->friendlist[friendnumber].userstatus = userstatus_from_int(status);
}

non_null()
Expand Down Expand Up @@ -2081,9 +2095,9 @@ static int m_handle_packet_userstatus(Messenger *m, const int i, const uint8_t *
return 0;
}

const Userstatus status = (Userstatus)data[0];
const Userstatus status = userstatus_from_int(data[0]);

if (status >= USERSTATUS_INVALID) {
if (status == USERSTATUS_INVALID) {
return 0;
}

Expand Down Expand Up @@ -3622,7 +3636,9 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
m->onion = new_onion(m->log, m->mem, m->mono_time, m->rng, m->dht);
m->onion_a = new_onion_announce(m->log, m->mem, m->rng, m->mono_time, m->dht);
m->onion_c = new_onion_client(m->log, m->mem, m->rng, m->mono_time, m->net_crypto);
m->fr_c = new_friend_connections(m->log, m->mono_time, m->ns, m->onion_c, options->local_discovery_enabled);
if (m->onion_c != nullptr) {
m->fr_c = new_friend_connections(m->log, m->mono_time, m->ns, m->onion_c, options->local_discovery_enabled);
}

if ((options->dht_announcements_enabled && (m->forwarding == nullptr || m->announce == nullptr)) ||
m->onion == nullptr || m->onion_a == nullptr || m->onion_c == nullptr || m->fr_c == nullptr) {
Expand Down
2 changes: 1 addition & 1 deletion toxcore/attributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#define GNU_PRINTF(f, a)
#endif

#if defined(__GNUC__) && defined(_DEBUG) && !defined(__OPTIMIZE__)
#if defined(__GNUC__) && defined(_DEBUG)
#define non_null(...) __attribute__((__nonnull__(__VA_ARGS__)))
#else
#define non_null(...)
Expand Down
6 changes: 4 additions & 2 deletions toxcore/crypto_core_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ TEST(CryptoCore, Signatures)

EXPECT_TRUE(create_extended_keypair(pk.data(), sk.data()));

std::vector<uint8_t> message;
std::vector<uint8_t> message{0};
message.clear();

// Try a few different sizes, including empty 0 length message.
for (uint8_t i = 0; i < 100; ++i) {
Expand All @@ -101,7 +102,8 @@ TEST(CryptoCore, Hmac)
HmacKey sk;
new_hmac_key(rng, sk.data());

std::vector<uint8_t> message;
std::vector<uint8_t> message{0};
message.clear();

// Try a few different sizes, including empty 0 length message.
for (uint8_t i = 0; i < 100; ++i) {
Expand Down
16 changes: 16 additions & 0 deletions toxcore/group_announce_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ TEST_F(Announces, CanBeCreatedAndDeleted)
GC_Public_Announce ann{};
ann.chat_public_key[0] = 0x88;
ASSERT_NE(gca_add_announce(mono_time_, gca_, &ann), nullptr);
#ifndef _DEBUG
ASSERT_EQ(gca_add_announce(mono_time_, gca_, nullptr), nullptr);
ASSERT_EQ(gca_add_announce(mono_time_, nullptr, &ann), nullptr);
#endif
}

TEST_F(Announces, AnnouncesCanTimeOut)
Expand Down Expand Up @@ -103,7 +105,9 @@ TEST_F(Announces, AnnouncesGetAndCleanup)

cleanup_gca(gca_, ann2.chat_public_key);
ASSERT_EQ(gca_get_announces(gca_, &announces, 1, ann2.chat_public_key, empty_pk), 0);
#ifndef _DEBUG
ASSERT_EQ(gca_get_announces(gca_, nullptr, 1, ann2.chat_public_key, empty_pk), -1);
#endif
}

struct AnnouncesPack : ::testing::Test {
Expand Down Expand Up @@ -162,19 +166,23 @@ TEST_F(AnnouncesPack, PublicAnnounceCanBePackedAndUnpacked)

TEST_F(AnnouncesPack, UnpackEmptyPublicAnnounce)
{
#ifndef _DEBUG
GC_Public_Announce ann{};
std::vector<uint8_t> packed(GCA_PUBLIC_ANNOUNCE_MAX_SIZE);

EXPECT_EQ(gca_unpack_public_announce(logger_, nullptr, 0, &ann), -1);
EXPECT_EQ(gca_unpack_public_announce(logger_, packed.data(), packed.size(), nullptr), -1);
#endif
}

TEST_F(AnnouncesPack, PackEmptyPublicAnnounce)
{
#ifndef _DEBUG
GC_Public_Announce ann{};
std::vector<uint8_t> packed(GCA_PUBLIC_ANNOUNCE_MAX_SIZE);
EXPECT_EQ(gca_pack_public_announce(logger_, packed.data(), packed.size(), nullptr), -1);
EXPECT_EQ(gca_pack_public_announce(logger_, nullptr, 0, &ann), -1);
#endif
}

TEST_F(AnnouncesPack, PublicAnnouncePackNull)
Expand All @@ -198,7 +206,9 @@ TEST_F(AnnouncesPack, PublicAnnouncePackNull)

TEST_F(AnnouncesPack, AnnouncesValidationCheck)
{
#ifndef _DEBUG
EXPECT_EQ(gca_is_valid_announce(nullptr), false);
#endif

GC_Announce announce = {0};
EXPECT_EQ(gca_is_valid_announce(&announce), false);
Expand All @@ -217,8 +227,10 @@ TEST_F(AnnouncesPack, UnpackIncompleteAnnouncesList)

GC_Announce announce;
EXPECT_EQ(gca_unpack_announces_list(logger_, data, sizeof(data), &announce, 1), -1);
#ifndef _DEBUG
EXPECT_EQ(gca_unpack_announces_list(logger_, data, sizeof(data), nullptr, 1), -1);
EXPECT_EQ(gca_unpack_announces_list(logger_, nullptr, 0, &announce, 1), -1);
#endif
}

TEST_F(AnnouncesPack, PackedAnnouncesListCanBeUnpacked)
Expand Down Expand Up @@ -246,17 +258,21 @@ TEST_F(AnnouncesPack, PackingEmptyAnnounceFails)
std::vector<uint8_t> packed(gca_pack_announces_list_size(1));
EXPECT_EQ(
gca_pack_announces_list(logger_, packed.data(), packed.size(), &announce, 1, nullptr), -1);
#ifndef _DEBUG
EXPECT_EQ(
gca_pack_announces_list(logger_, packed.data(), packed.size(), nullptr, 1, nullptr), -1);
EXPECT_EQ(gca_pack_announces_list(logger_, nullptr, 0, &announce, 1, nullptr), -1);
#endif
}

TEST_F(AnnouncesPack, PackAnnounceNull)
{
#ifndef _DEBUG
std::vector<uint8_t> data(GCA_ANNOUNCE_MAX_SIZE);
GC_Announce announce;
ASSERT_EQ(gca_pack_announce(logger_, nullptr, 0, &announce), -1);
ASSERT_EQ(gca_pack_announce(logger_, data.data(), data.size(), nullptr), -1);
#endif
}

} // namespace
6 changes: 3 additions & 3 deletions toxcore/group_chats.c
Original file line number Diff line number Diff line change
Expand Up @@ -1270,8 +1270,8 @@ static uint16_t unpack_gc_shared_state(GC_SharedState *shared_state, const uint8
memcpy(&voice_state, data + len_processed, sizeof(uint8_t));
len_processed += sizeof(uint8_t);

shared_state->voice_state = (Group_Voice_State)voice_state;
shared_state->privacy_state = (Group_Privacy_State)privacy_state;
shared_state->voice_state = group_voice_state_from_int(voice_state);
shared_state->privacy_state = group_privacy_state_from_int(privacy_state);

return len_processed;
}
Expand Down Expand Up @@ -7279,7 +7279,7 @@ static bool init_gc_tcp_connection(const GC_Session *c, GC_Chat *chat)

/** Initializes default shared state values. */
non_null()
static void init_gc_shared_state(GC_Chat *chat, const Group_Privacy_State privacy_state)
static void init_gc_shared_state(GC_Chat *chat, Group_Privacy_State privacy_state)
{
chat->shared_state.maxpeers = MAX_GC_PEERS_DEFAULT;
chat->shared_state.privacy_state = privacy_state;
Expand Down
2 changes: 1 addition & 1 deletion toxcore/group_chats.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ uint16_t gc_get_wrapped_packet_size(uint16_t length, Net_Packet_Type packet_type
* Returns -4 if the sender does not have permission to speak.
* Returns -5 if the packet fails to send.
*/
non_null(1, 2, 3, 4) nullable(5)
non_null(1, 2) nullable(5)
int gc_send_message(const GC_Chat *chat, const uint8_t *message, uint16_t length, uint8_t type,
uint32_t *message_id);

Expand Down
30 changes: 28 additions & 2 deletions toxcore/group_pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,32 @@
#include "ccompat.h"
#include "util.h"

Group_Privacy_State group_privacy_state_from_int(uint8_t value)
{
switch (value) {
case 0:
return GI_PUBLIC;
case 1:
return GI_PRIVATE;
default:
return GI_PUBLIC;
}
}

Group_Voice_State group_voice_state_from_int(uint8_t value)
{
switch (value) {
case 0:
return GV_ALL;
case 1:
return GV_MODS;
case 2:
return GV_FOUNDER;
default:
return GV_ALL;
}
}

non_null()
static bool load_unpack_state_values(GC_Chat *chat, Bin_Unpack *bu)
{
Expand All @@ -44,8 +70,8 @@ static bool load_unpack_state_values(GC_Chat *chat, Bin_Unpack *bu)
}

chat->connection_state = manually_disconnected ? CS_DISCONNECTED : CS_CONNECTING;
chat->shared_state.privacy_state = (Group_Privacy_State)privacy_state;
chat->shared_state.voice_state = (Group_Voice_State)voice_state;
chat->shared_state.privacy_state = group_privacy_state_from_int(privacy_state);
chat->shared_state.voice_state = group_voice_state_from_int(voice_state);

// we always load saved groups as private in case the group became private while we were offline.
// this will have no detrimental effect if the group is public, as the correct privacy
Expand Down
3 changes: 3 additions & 0 deletions toxcore/group_pack.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,7 @@ void gc_save_pack_group(const GC_Chat *chat, Bin_Pack *bp);
non_null()
bool gc_load_unpack_group(GC_Chat *chat, Bin_Unpack *bu);

Group_Privacy_State group_privacy_state_from_int(uint8_t value);
Group_Voice_State group_voice_state_from_int(uint8_t value);

#endif // GROUP_PACK_H
Loading

0 comments on commit 3983369

Please sign in to comment.