osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

gboolean bitfields considered harmful 

ISO C Std § 6.7.2, 5: "for bit-fields, it is implementation-defined
whether the specifier int designates the same type as signed int or the
same type as unsigned int." (See also the note in § 6.7.2.1 and ISO C
Std Appendix J.3.9.)

A gboolean is a typedef'd gint. Therefore, many implementations,
including gcc and clang, treat a gboolean bitfield of width 1 as
signed, meaning that it has two possible values: 0 and -1, any time
the integer promotions occur (which is all the time.) Constructs like this:

        dgram_info->from_server = TRUE;
        if (dgram_info->from_server == TRUE) ws_warning("True");

will not work as expected, though gcc (but not clang) will give an
error:

/home/johnthacker/wireshark/epan/dissectors/packet-quic.c:3457:37: error: comparison is always false due to limited range of data type [-Werror=type-limits]
 3457 |         if (dgram_info->from_server == TRUE)
      |

        proto_tree_add_debug_text(quic_tree, "Connection: %d %p from_server:%d", pinfo->num, dgram_info->conn, dgram_info->from_server);

Connection: 1 0x7fc4b47f2be0 from_server:0
Connection: 2 0x7fc4b47f2be0 from_server:-1
Connection: 3 0x7fc4b47f2be0 from_server:0
Connection: 4 0x7fc4b47f2be0 from_server:-1

At worst this can cause buffer overruns.

If a bitfield is desired, to guarantee expected behavior the standard
_Bool/bool should be used instead.
This commit is contained in:
John Thacker 2022-07-30 08:49:08 -04:00
parent 735ae00417
commit 5aba5772e9
3 changed files with 22 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
epan/dissectors/packet-quic.c
View File

@ -48,6 +48,8 @@
#include <config.h>
#include <stdbool.h>
#include <epan/packet.h>
#include <epan/expert.h>
#include <epan/proto_data.h>
@ -346,7 +348,7 @@ typedef struct quic_pp_state {
quic_pp_cipher pp_ciphers[2]; /**< PP cipher for Key Phase 0/1 */
quic_hp_cipher hp_cipher; /**< HP cipher for both Key Phases; it does not change after KeyUpdate */
guint64 changed_in_pkn; /**< Packet number where key change occurred. */
gboolean key_phase : 1; /**< Current key phase. */
bool key_phase : 1; /**< Current key phase. */
} quic_pp_state_t;
/** Singly-linked list of Connection IDs. */
@ -402,12 +404,12 @@ typedef struct quic_info_data {
guint32 version;
address server_address;
guint16 server_port;
gboolean skip_decryption : 1; /**< Set to 1 if no keys are available. */
gboolean client_dcid_set : 1; /**< Set to 1 if client_dcid_initial is set. */
gboolean client_loss_bits_recv : 1; /**< The client is able to read loss bits info */
gboolean client_loss_bits_send : 1; /**< The client wants to send loss bits info */
gboolean server_loss_bits_recv : 1; /**< The server is able to read loss bits info */
gboolean server_loss_bits_send : 1; /**< The server wants to send loss bits info */
bool skip_decryption : 1; /**< Set to 1 if no keys are available. */
bool client_dcid_set : 1; /**< Set to 1 if client_dcid_initial is set. */
bool client_loss_bits_recv : 1; /**< The client is able to read loss bits info */
bool client_loss_bits_send : 1; /**< The client wants to send loss bits info */
bool server_loss_bits_recv : 1; /**< The server is able to read loss bits info */
bool server_loss_bits_send : 1; /**< The server wants to send loss bits info */
int hash_algo; /**< Libgcrypt hash algorithm for key derivation. */
int cipher_algo; /**< Cipher algorithm for packet number and packet encryption. */
int cipher_mode; /**< Cipher mode for packet encryption. */
@ -448,8 +450,8 @@ struct quic_packet_info {
guint8 pkn_len; /**< Length of PKN (1/2/3/4) or unknown (0). */
guint8 first_byte; /**< Decrypted flag byte, valid only if pkn_len is non-zero. */
guint8 packet_type;
gboolean retry_integrity_failure : 1;
gboolean retry_integrity_success : 1;
bool retry_integrity_failure : 1;
bool retry_integrity_success : 1;
};
typedef struct quic_packet_info quic_packet_info_t;
@ -457,7 +459,7 @@ typedef struct quic_packet_info quic_packet_info_t;
typedef struct quic_datagram {
quic_info_data_t *conn;
quic_packet_info_t first_packet;
gboolean from_server : 1;
bool from_server : 1;
} quic_datagram;
/**

5
epan/dissectors/packet-wireguard.c
View File

@ -16,6 +16,7 @@
#include <config.h>
#include <errno.h>
#include <stdbool.h>
#define WS_LOG_DOMAIN "packet-wireguard"
@ -210,8 +211,8 @@ typedef struct {
const wg_skey_t *initiator_skey; /* Spub_i based on Initiation.static (decrypted, null if decryption failed) */
const wg_skey_t *responder_skey; /* Spub_r based on Initiation.MAC1 (+Spriv_r if available) */
guint8 timestamp[12]; /* Initiation.timestamp (decrypted) */
gboolean timestamp_ok : 1; /* Whether the timestamp was successfully decrypted */
gboolean empty_ok : 1; /* Whether the empty field was successfully decrypted */
bool timestamp_ok : 1; /* Whether the timestamp was successfully decrypted */
bool empty_ok : 1; /* Whether the empty field was successfully decrypted */
/* The following fields are only valid on the initial pass. */
const wg_ekey_t *initiator_ekey; /* Epub_i matching Initiation.Ephemeral (+Epriv_i if available) */

13
ui/tap-sctp-analysis.h
View File

@ -12,6 +12,7 @@
#ifndef __TAP_SCTP_ANALYSIS_H__
#define __TAP_SCTP_ANALYSIS_H__
#include <stdbool.h>
#include <epan/dissectors/packet-sctp.h>
#include <epan/address.h>
#ifdef _WIN32
@ -165,8 +166,8 @@ typedef struct _sctp_init_collision {
guint32 initack_vtag; /* initiate tag of the INIT-ACK chunk */
guint32 init_min_tsn; /* initial tsn of the INIT chunk */
guint32 initack_min_tsn; /* initial tsn of the INIT-ACK chunk */
gboolean init:1;
gboolean initack:1;
bool init:1;
bool initack:1;
} sctp_init_collision_t;
struct tsn_sort{
@ -232,10 +233,10 @@ typedef struct _sctp_assoc_info {
guint32 max_window2;
guint32 arwnd1;
guint32 arwnd2;
gboolean init:1;
gboolean initack:1;
gboolean firstdata:1;
gboolean init_collision:1;
bool init:1;
bool initack:1;
bool firstdata:1;
bool init_collision:1;
guint16 initack_dir;
guint16 direction;
guint32 min_secs;