Skip to content

Commit

Permalink
cleanup: Use memzero(x, s) instead of memset(x, 0, s).
Browse files Browse the repository at this point in the history
It's clearer and doesn't risk having a non-zero filler value.
  • Loading branch information
iphydf committed Jan 24, 2024
1 parent a7258e4 commit 8f07755
Show file tree
Hide file tree
Showing 20 changed files with 105 additions and 66 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 @@
9190d56ef3b346bc1632e6d7ee5fe5362be661bff58f2d6d88b5c1d1827394cd /usr/local/bin/tox-bootstrapd
c028527fe5eecde7daa6ac9dacc47bd4bb765463e8e4b5af3881789797bf81a8 /usr/local/bin/tox-bootstrapd
1 change: 1 addition & 0 deletions other/docker/compcert/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
!/Makefile
28 changes: 10 additions & 18 deletions other/docker/compcert/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
FROM toxchat/compcert:latest

RUN apt-get update && \
DEBIAN_FRONTEND="noninteractive" apt-get install -y --no-install-recommends \
gdb \
make \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*

WORKDIR /work
COPY auto_tests/ /work/auto_tests/
COPY testing/ /work/testing/
Expand All @@ -10,21 +17,6 @@ COPY third_party/ /work/third_party/

SHELL ["/bin/bash", "-o", "pipefail", "-c"]

RUN ccomp \
-o send_message_test \
-Wall -Werror \
-Wno-c11-extensions \
-Wno-unknown-pragmas \
-Wno-unused-variable \
-fstruct-passing -fno-unprototyped -g \
auto_tests/auto_test_support.c \
auto_tests/send_message_test.c \
testing/misc_tools.c \
toxav/*.c \
toxcore/*.c \
toxcore/*/*.c \
toxencryptsave/*.c \
third_party/cmp/*.c \
-D__COMPCERT__ -DDISABLE_VLA -Dinline= \
-lpthread $(pkg-config --cflags --libs libsodium opus vpx) \
&& ./send_message_test | grep 'tox clients connected'
COPY other/docker/compcert/Makefile /work/
RUN make "-j$(nproc)"
RUN ./send_message_test #| grep 'tox clients connected'
26 changes: 26 additions & 0 deletions other/docker/compcert/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
SOURCES := $(wildcard auto_tests/auto_test_support.c \
auto_tests/send_message_test.c \
testing/misc_tools.c \
toxav/*.c \
toxcore/*.c \
toxcore/*/*.c \
toxencryptsave/*.c \
third_party/cmp/*.c)

OBJECTS := $(SOURCES:.c=.o)

CC := ccomp
CFLAGS := -Wall -Werror \
-Wno-c11-extensions \
-Wno-unknown-pragmas \
-Wno-unused-variable \
-fstruct-passing -fno-unprototyped -g \
-D__COMPCERT__ \
-DDISABLE_VLA \
-DMIN_LOGGER_LEVEL=LOGGER_LEVEL_TRACE \
-Dinline= \
$(shell pkg-config --cflags libsodium opus vpx)
LDFLAGS := -lpthread $(shell pkg-config --libs libsodium opus vpx)

send_message_test: $(OBJECTS)
$(CC) -o $@ $+ $(LDFLAGS)
1 change: 1 addition & 0 deletions toxcore/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ cc_library(
deps = [
":attributes",
":ccompat",
":util",
"@libsodium",
],
)
Expand Down
11 changes: 7 additions & 4 deletions toxcore/DHT.c
Original file line number Diff line number Diff line change
Expand Up @@ -853,12 +853,14 @@ static void get_close_nodes_inner(
*/
non_null()
static int get_somewhat_close_nodes(
uint64_t cur_time, const uint8_t *public_key, Node_format *nodes_list,
uint64_t cur_time, const uint8_t *public_key, Node_format nodes_list[MAX_SENT_NODES],
Family sa_family, const Client_data *close_clientlist,
const DHT_Friend *friends_list, uint16_t friends_list_size,
bool is_lan, bool want_announce)
{
memset(nodes_list, 0, MAX_SENT_NODES * sizeof(Node_format));
for (uint16_t i = 0; i < MAX_SENT_NODES; ++i) {
nodes_list[i] = empty_node_format;
}

uint32_t num_nodes = 0;
get_close_nodes_inner(
Expand All @@ -882,7 +884,7 @@ static int get_somewhat_close_nodes(

int get_close_nodes(
const DHT *dht, const uint8_t *public_key,
Node_format *nodes_list, Family sa_family,
Node_format nodes_list[MAX_SENT_NODES], Family sa_family,
bool is_lan, bool want_announce)
{
return get_somewhat_close_nodes(
Expand Down Expand Up @@ -1101,7 +1103,8 @@ static void update_client_with_reset(const Mono_Time *mono_time, Client_data *cl
ipptp_write->ret_ip_self = false;

/* zero out other address */
memset(ipptp_clear, 0, sizeof(*ipptp_clear));
const IPPTsPng empty_ipptp = {{{{0}}}};
*ipptp_clear = empty_ipptp;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion toxcore/DHT.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ void set_announce_node(DHT *dht, const uint8_t *public_key);
non_null()
int get_close_nodes(
const DHT *dht, const uint8_t *public_key,
Node_format *nodes_list, Family sa_family,
Node_format nodes_list[MAX_SENT_NODES], Family sa_family,
bool is_lan, bool want_announce);


Expand Down
4 changes: 1 addition & 3 deletions toxcore/LAN_discovery.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "LAN_discovery.h"

#include <stdlib.h>
#include <string.h>

#if defined(_WIN32) || defined(__WIN32__) || defined(WIN32)
// The mingw32/64 Windows library warns about including winsock2.h after
Expand Down Expand Up @@ -143,8 +142,7 @@ static Broadcast_Info *fetch_broadcast_info(const Network *ns)
}

/* Configure ifconf for the ioctl call. */
struct ifreq i_faces[MAX_INTERFACES];
memset(i_faces, 0, sizeof(struct ifreq) * MAX_INTERFACES);
struct ifreq i_faces[MAX_INTERFACES] = {{0}};

struct ifconf ifc;
ifc.ifc_buf = (char *)i_faces;
Expand Down
7 changes: 3 additions & 4 deletions toxcore/Messenger.c
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ int m_copy_statusmessage(const Messenger *m, int32_t friendnumber, uint8_t *buf,
const uint32_t msglen = min_u32(maxlen, m->friendlist[friendnumber].statusmessage_length);

memcpy(buf, m->friendlist[friendnumber].statusmessage, msglen);
memset(buf + msglen, 0, maxlen - msglen);
memzero(buf + msglen, maxlen - msglen);
return msglen;
}

Expand Down Expand Up @@ -2620,7 +2620,7 @@ static bool self_announce_group(const Messenger *m, GC_Chat *chat, Onion_Friend
if (tcp_num > 0) {
pk_copy(chat->announced_tcp_relay_pk, announce.base_announce.tcp_relays[0].public_key);
} else {
memset(chat->announced_tcp_relay_pk, 0, sizeof(chat->announced_tcp_relay_pk));
memzero(chat->announced_tcp_relay_pk, sizeof(chat->announced_tcp_relay_pk));
}

LOGGER_DEBUG(chat->log, "Published group announce. TCP relays: %d, UDP status: %d", tcp_num,
Expand Down Expand Up @@ -3416,10 +3416,9 @@ static uint32_t path_node_size(const Messenger *m)
non_null()
static uint8_t *save_path_nodes(const Messenger *m, uint8_t *data)
{
Node_format nodes[NUM_SAVED_PATH_NODES];
Node_format nodes[NUM_SAVED_PATH_NODES] = {{{0}}};
uint8_t *temp_data = data;
data = state_write_section_header(data, STATE_COOKIE_TYPE, 0, STATE_TYPE_PATH_NODE);
memset(nodes, 0, sizeof(nodes));
const unsigned int num = onion_backup_nodes(m->onion_c, nodes, NUM_SAVED_PATH_NODES);
const int l = pack_nodes(m->log, data, NUM_SAVED_PATH_NODES * packed_node_size(net_family_tcp_ipv6()), nodes, num);

Expand Down
7 changes: 5 additions & 2 deletions toxcore/TCP_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ typedef struct TCP_Secure_Connection {
uint64_t ping_id;
} TCP_Secure_Connection;

static const TCP_Secure_Connection empty_tcp_secure_connection = {{nullptr}};


struct TCP_Server {
const Logger *logger;
Expand Down Expand Up @@ -135,8 +137,9 @@ static int alloc_new_connections(TCP_Server *tcp_server, uint32_t num)
}

const uint32_t old_size = tcp_server->size_accepted_connections;
const uint32_t size_new_entries = num * sizeof(TCP_Secure_Connection);
memset(new_connections + old_size, 0, size_new_entries);
for (uint32_t i = 0; i < num; ++i) {
new_connections[old_size + i] = empty_tcp_secure_connection;
}

tcp_server->accepted_connection_array = new_connections;
tcp_server->size_accepted_connections = new_size;
Expand Down
19 changes: 10 additions & 9 deletions toxcore/crypto_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <sodium.h>

#include "ccompat.h"
#include "util.h"

#ifndef crypto_box_MACBYTES
#define crypto_box_MACBYTES (crypto_box_ZEROBYTES - crypto_box_BOXZEROBYTES)
Expand Down Expand Up @@ -122,7 +123,7 @@ static void crypto_free(uint8_t *ptr, size_t bytes)
void crypto_memzero(void *data, size_t length)
{
#if defined(FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION)
memset(data, 0, length);
memzero(data, length);
#else
sodium_memzero(data, length);
#endif /* FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */
Expand Down Expand Up @@ -264,7 +265,7 @@ int32_t encrypt_data_symmetric(const uint8_t *shared_key, const uint8_t *nonce,
// Don't encrypt anything.
memcpy(encrypted, plain, length);
// Zero MAC to avoid uninitialized memory reads.
memset(encrypted + length, 0, crypto_box_MACBYTES);
memzero(encrypted + length, crypto_box_MACBYTES);

Check warning on line 268 in toxcore/crypto_core.c

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

toxcore/crypto_core.c#L268

MISRA 18.4 rule
#else

const size_t size_temp_plain = length + crypto_box_ZEROBYTES;
Expand All @@ -282,9 +283,9 @@ int32_t encrypt_data_symmetric(const uint8_t *shared_key, const uint8_t *nonce,
// crypto_box_afternm requires the entire range of the output array be
// initialised with something. It doesn't matter what it's initialised with,
// so we'll pick 0x00.
memset(temp_encrypted, 0, size_temp_encrypted);
memzero(temp_encrypted, size_temp_encrypted);

memset(temp_plain, 0, crypto_box_ZEROBYTES);
memzero(temp_plain, crypto_box_ZEROBYTES);
// Pad the message with 32 0 bytes.
memcpy(temp_plain + crypto_box_ZEROBYTES, plain, length);

Expand Down Expand Up @@ -333,9 +334,9 @@ int32_t decrypt_data_symmetric(const uint8_t *shared_key, const uint8_t *nonce,
// crypto_box_open_afternm requires the entire range of the output array be
// initialised with something. It doesn't matter what it's initialised with,
// so we'll pick 0x00.
memset(temp_plain, 0, size_temp_plain);
memzero(temp_plain, size_temp_plain);

memset(temp_encrypted, 0, crypto_box_BOXZEROBYTES);
memzero(temp_encrypted, crypto_box_BOXZEROBYTES);
// Pad the message with 16 0 bytes.
memcpy(temp_encrypted + crypto_box_BOXZEROBYTES, encrypted, length);

Expand Down Expand Up @@ -438,7 +439,7 @@ void new_symmetric_key(const Random *rng, uint8_t *key)
int32_t crypto_new_keypair(const Random *rng, uint8_t *public_key, uint8_t *secret_key)
{
random_bytes(rng, secret_key, CRYPTO_SECRET_KEY_SIZE);
memset(public_key, 0, CRYPTO_PUBLIC_KEY_SIZE); // Make MSAN happy
memzero(public_key, CRYPTO_PUBLIC_KEY_SIZE); // Make MSAN happy
crypto_derive_public_key(public_key, secret_key);
return 0;
}
Expand Down Expand Up @@ -477,7 +478,7 @@ bool crypto_hmac_verify(const uint8_t auth[CRYPTO_HMAC_SIZE], const uint8_t key[
void crypto_sha256(uint8_t *hash, const uint8_t *data, size_t length)
{
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
memset(hash, 0, CRYPTO_SHA256_SIZE);
memzero(hash, CRYPTO_SHA256_SIZE);
memcpy(hash, data, length < CRYPTO_SHA256_SIZE ? length : CRYPTO_SHA256_SIZE);
#else
crypto_hash_sha256(hash, data, length);
Expand All @@ -487,7 +488,7 @@ void crypto_sha256(uint8_t *hash, const uint8_t *data, size_t length)
void crypto_sha512(uint8_t *hash, const uint8_t *data, size_t length)
{
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
memset(hash, 0, CRYPTO_SHA512_SIZE);
memzero(hash, CRYPTO_SHA512_SIZE);
memcpy(hash, data, length < CRYPTO_SHA512_SIZE ? length : CRYPTO_SHA512_SIZE);
#else
crypto_hash_sha512(hash, data, length);
Expand Down
3 changes: 2 additions & 1 deletion toxcore/friend_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "net_crypto.h"
#include "network.h"
#include "onion_client.h"
#include "util.h"

#define PORTS_PER_DISCOVERY 10

Expand Down Expand Up @@ -982,7 +983,7 @@ void do_friend_connections(Friend_Connections *fr_c, void *userdata)
if (friend_con->dht_lock_token > 0) {
dht_delfriend(fr_c->dht, friend_con->dht_temp_pk, friend_con->dht_lock_token);
friend_con->dht_lock_token = 0;
memset(friend_con->dht_temp_pk, 0, CRYPTO_PUBLIC_KEY_SIZE);
memzero(friend_con->dht_temp_pk, CRYPTO_PUBLIC_KEY_SIZE);
}
}

Expand Down
15 changes: 6 additions & 9 deletions toxcore/group_chats.c
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ static bool set_gc_password_local(GC_Chat *chat, const uint8_t *passwd, uint16_t

if (passwd == nullptr || length == 0) {
chat->shared_state.password_length = 0;
memset(chat->shared_state.password, 0, MAX_GC_PASSWORD_SIZE);
memzero(chat->shared_state.password, MAX_GC_PASSWORD_SIZE);
} else {
chat->shared_state.password_length = length;
crypto_memlock(chat->shared_state.password, sizeof(chat->shared_state.password));
Expand Down Expand Up @@ -1526,7 +1526,7 @@ int group_packet_wrap(

assert(padding_len < packet_size);

memset(plain, 0, padding_len);
memzero(plain, padding_len);

uint16_t enc_header_len = sizeof(uint8_t);
plain[padding_len] = gp_packet_type;
Expand Down Expand Up @@ -2492,8 +2492,7 @@ static int handle_gc_ping(GC_Chat *chat, GC_Connection *gconn, const uint8_t *da
do_gc_peer_state_sync(chat, gconn, data, length);

if (length > GC_PING_PACKET_MIN_DATA_SIZE) {
IP_Port ip_port;
memset(&ip_port, 0, sizeof(IP_Port));
IP_Port ip_port = {{{0}}};

if (unpack_ip_port(&ip_port, data + GC_PING_PACKET_MIN_DATA_SIZE,
length - GC_PING_PACKET_MIN_DATA_SIZE, false) > 0) {
Expand Down Expand Up @@ -3792,7 +3791,7 @@ int gc_set_topic(GC_Chat *chat, const uint8_t *topic, uint16_t length)
assert(topic != nullptr);
memcpy(chat->topic_info.topic, topic, length);
} else {
memset(chat->topic_info.topic, 0, sizeof(chat->topic_info.topic));
memzero(chat->topic_info.topic, sizeof(chat->topic_info.topic));
}

memcpy(chat->topic_info.public_sig_key, get_sig_pk(chat->self_public_key), SIG_PUBLIC_KEY_SIZE);
Expand Down Expand Up @@ -5622,8 +5621,7 @@ static bool send_gc_handshake_packet(const GC_Chat *chat, GC_Connection *gconn,
return false;
}

Node_format node;
memset(&node, 0, sizeof(node));
Node_format node = {{0}};

if (!gcc_copy_tcp_relay(chat->rng, &node, gconn)) {
LOGGER_TRACE(chat->log, "Failed to copy TCP relay during handshake (%u TCP relays)", gconn->tcp_relays_count);
Expand Down Expand Up @@ -5678,8 +5676,7 @@ static bool send_gc_oob_handshake_request(const GC_Chat *chat, const GC_Connecti
return false;
}

Node_format node;
memset(&node, 0, sizeof(node));
Node_format node = {{0}};

if (!gcc_copy_tcp_relay(chat->rng, &node, gconn)) {
LOGGER_WARNING(chat->log, "Failed to copy TCP relay");
Expand Down
4 changes: 2 additions & 2 deletions toxcore/group_moderation.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ void mod_list_get_data_hash(uint8_t *hash, const uint8_t *packed_mod_list, uint1
bool mod_list_make_hash(const Moderation *moderation, uint8_t *hash)
{
if (moderation->num_mods == 0) {
memset(hash, 0, MOD_MODERATION_HASH_SIZE);
memzero(hash, MOD_MODERATION_HASH_SIZE);
return true;
}

Expand Down Expand Up @@ -410,7 +410,7 @@ static bool sanctions_list_make_hash(const Mod_Sanction *sanctions, uint32_t new
uint8_t *hash)
{
if (num_sanctions == 0 || sanctions == nullptr) {
memset(hash, 0, MOD_SANCTION_HASH_SIZE);
memzero(hash, MOD_SANCTION_HASH_SIZE);
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions toxcore/net_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ static int create_cookie_request(const Net_Crypto *c, uint8_t *packet, const uin
uint8_t plain[COOKIE_REQUEST_PLAIN_LENGTH];

memcpy(plain, c->self_public_key, CRYPTO_PUBLIC_KEY_SIZE);
memset(plain + CRYPTO_PUBLIC_KEY_SIZE, 0, CRYPTO_PUBLIC_KEY_SIZE);
memzero(plain + CRYPTO_PUBLIC_KEY_SIZE, CRYPTO_PUBLIC_KEY_SIZE);
memcpy(plain + (CRYPTO_PUBLIC_KEY_SIZE * 2), &number, sizeof(uint64_t));
const uint8_t *tmp_shared_key = dht_get_shared_key_sent(c->dht, dht_public_key);
memcpy(shared_key, tmp_shared_key, CRYPTO_SHARED_KEY_SIZE);
Expand Down Expand Up @@ -1139,7 +1139,7 @@ static int send_data_packet_helper(Net_Crypto *c, int crypt_connection_id, uint3
VLA(uint8_t, packet, sizeof(uint32_t) + sizeof(uint32_t) + padding_length + length);
memcpy(packet, &buffer_start, sizeof(uint32_t));
memcpy(packet + sizeof(uint32_t), &num, sizeof(uint32_t));
memset(packet + (sizeof(uint32_t) * 2), 0, padding_length);
memzero(packet + (sizeof(uint32_t) * 2), padding_length);
memcpy(packet + (sizeof(uint32_t) * 2) + padding_length, data, length);

return send_data_packet(c, crypt_connection_id, packet, SIZEOF_VLA(packet));
Expand Down
Loading

0 comments on commit 8f07755

Please sign in to comment.