1
0
mirror of https://github.com/unrealircd/unrealircd.git synced 2026-06-12 17:14:46 +02:00

We now compile with -Wformat-nonliteral by default.

This adds __attribute__((format(printf,X,Y))) to several functions.
It also adds checking only for the non-literal case to some functions
such as unreal_log/unreal_do_log.

This so we can more easily detect format string issues. Especially now with
the recoding of the logger and with possible future mistakes in this area
in UnrealIRCd 6 itself or in third party modules.

The check is currently disabled in these files, which are TODO items:
* src/send.c: still much work to do
* src/socket.c: due to report_error and report_baderror().
  I want to get rid of these functions and integrate them
  in the new logger anyway.
* src/serv.c: only disable for hunt_server()
This commit is contained in:
Bram Matthys
2021-07-14 11:06:40 +02:00
parent 01c418c4d5
commit 1068960b9a
13 changed files with 89 additions and 37 deletions
Vendored
+48
View File
@@ -5229,6 +5229,54 @@ ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
ac_compiler_gnu=$ac_cv_c_compiler_gnu
ac_ext=c
ac_cpp='$CPP $CPPFLAGS'
ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
ac_compiler_gnu=$ac_cv_c_compiler_gnu
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether C compiler accepts -Wformat-nonliteral" >&5
$as_echo_n "checking whether C compiler accepts -Wformat-nonliteral... " >&6; }
if ${ax_cv_check_cflags__Werror___Wformat_nonliteral+:} false; then :
$as_echo_n "(cached) " >&6
else
ax_check_save_flags=$CFLAGS
CFLAGS="$CFLAGS -Werror -Wformat-nonliteral"
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
int
main ()
{
;
return 0;
}
_ACEOF
if ac_fn_c_try_compile "$LINENO"; then :
ax_cv_check_cflags__Werror___Wformat_nonliteral=yes
else
ax_cv_check_cflags__Werror___Wformat_nonliteral=no
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
CFLAGS=$ax_check_save_flags
fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_check_cflags__Werror___Wformat_nonliteral" >&5
$as_echo "$ax_cv_check_cflags__Werror___Wformat_nonliteral" >&6; }
if test x"$ax_cv_check_cflags__Werror___Wformat_nonliteral" = xyes; then :
CFLAGS="$CFLAGS -Wformat-nonliteral"
else
:
fi
ac_ext=c
ac_cpp='$CPP $CPPFLAGS'
ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
ac_compiler_gnu=$ac_cv_c_compiler_gnu
ac_ext=c
ac_cpp='$CPP $CPPFLAGS'
ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+2
View File
@@ -195,6 +195,8 @@ CFLAGS="$CFLAGS -Wall"
dnl More warnings (if the compiler supports it):
check_cc_flag([-Wextra], [CFLAGS="$CFLAGS -Wextra"])
check_cc_flag([-Waggregate-return], [CFLAGS="$CFLAGS -Waggregate-return"])
check_cc_flag([-Wformat-nonliteral], [CFLAGS="$CFLAGS -Wformat-nonliteral"])
dnl The following few are more experimental, if they have false positives we'll have
dnl to disable them:
dnl Can't use this, too bad: check_cc_flag([-Wlogical-op], [CFLAGS="$CFLAGS -Wlogical-op"])
+2 -2
View File
@@ -1024,7 +1024,7 @@ extern void user_account_login(MessageTag *recv_mtags, Client *client);
extern void link_generator(void);
extern void update_throttling_timer_settings(void);
extern int hide_idle_time(Client *client, Client *target);
extern void lost_server_link(Client *serv, FORMAT_STRING(const char *fmt), ...);
extern void lost_server_link(Client *serv, FORMAT_STRING(const char *fmt), ...) __attribute__((format(printf,2,3)));
extern char *sendtype_to_cmd(SendType sendtype);
extern MODVAR MessageTagHandler *mtaghandlers;
extern int security_group_valid_name(char *name);
@@ -1089,7 +1089,7 @@ extern char *log_type_valtostring(LogType v);
#else
#define unreal_log(...) do_unreal_log(__VA_ARGS__, NULL)
#endif
extern void do_unreal_log(LogLevel loglevel, char *subsystem, char *event_id, Client *client, char *msg, ...);
extern void do_unreal_log(LogLevel loglevel, char *subsystem, char *event_id, Client *client, char *msg, ...) __attribute__((format(printf,5,0)));
extern LogData *log_data_string(const char *key, const char *str);
extern LogData *log_data_integer(const char *key, int64_t integer);
extern LogData *log_data_client(const char *key, Client *client);
+1 -1
View File
@@ -17,7 +17,7 @@
# define FORMAT_STRING(p) p
#endif
extern char *ircvsnprintf(char *str, size_t size, const char *format, va_list);
extern char *ircvsnprintf(char *str, size_t size, const char *format, va_list) __attribute__((format(printf,3,0)));
extern char *ircsnprintf(char *str, size_t size, FORMAT_STRING(const char *format), ...) __attribute__((format(printf,3,4)));
#endif
-15
View File
@@ -502,21 +502,6 @@ char chess[] = {
85, 110, 114, 101, 97, 108, 0
};
static void version_check_logerror(char *fmt, ...)
{
va_list va;
char buf[1024];
va_start(va, fmt);
vsnprintf(buf, sizeof(buf), fmt, va);
va_end(va);
#ifndef _WIN32
fprintf(stderr, "[!!!] %s\n", buf);
#else
win_log("[!!!] %s", buf);
#endif
}
extern void applymeblock(void);
extern MODVAR Event *events;
+4 -2
View File
@@ -319,6 +319,8 @@ char *loglevel_to_string(LogLevel loglevel)
}
}
#define validvarcharacter(x) (isalnum((x)) || ((x) == '_'))
/** Build a string and replace $variables where needed.
* See src/modules/blacklist.c for an example.
* @param inbuf The input string
@@ -351,7 +353,7 @@ void buildlogstring(const char *inbuf, char *outbuf, size_t len, json_t *details
if (*i == '$')
goto literal;
if (!isalnum(*i))
if (!validvarcharacter(*i))
{
/* What do we do with things like '$/' ? -- treat literal */
i--;
@@ -359,7 +361,7 @@ void buildlogstring(const char *inbuf, char *outbuf, size_t len, json_t *details
}
/* find termination */
for (p=i; isalnum(*p) || ((*p == '.') && isalnum(p[1])); p++);
for (p=i; validvarcharacter(*p) || ((*p == '.') && validvarcharacter(p[1])); p++);
/* find variable name in list */
*varname = '\0';
+2 -2
View File
@@ -114,8 +114,8 @@ void ircd_log(int flags, FORMAT_STRING(const char *format), ...)
ircvsnprintf(buf, sizeof(buf), format, vl);
va_end(vl);
// FIXME: escape 'buf' !!!
do_unreal_log(ULOG_ERROR, "unknown", "UNKNOWN", NULL, buf, NULL);
// This is a stupid escape trick, we better use a different method / other function
unreal_log(ULOG_ERROR, "unknown", "UNKNOWN", NULL, "$_data", log_data_string("_data", buf));
}
/** Returns the date in rather long string */
+11 -12
View File
@@ -1035,11 +1035,11 @@ int stats_linkinfoall(Client *client, char *para)
int stats_linkinfoint(Client *client, char *para, int all)
{
#ifndef DEBUGMODE
static char Sformat[] = "SendQ SendM SendBytes RcveM RcveBytes Open_since :Idle";
static char Lformat[] = "%s%s %u %u %u %u %u %u :%u";
#define Sformat "SendQ SendM SendBytes RcveM RcveBytes Open_since :Idle"
#define Lformat "%s%s %u %u %u %u %u %lld :%lld"
#else
static char Sformat[] = "SendQ SendM SendBytes RcveM RcveBytes Open_since CPU :Idle";
static char Lformat[] = "%s%s %u %u %u %u %u %u %s";
#define Sformat "SendQ SendM SendBytes RcveM RcveBytes Open_since CPU :Idle"
#define Lformat "%s%s %u %u %u %u %u %lld %s"
char pbuf[96]; /* Should be enough for to ints */
#endif
int remote = 0;
@@ -1090,8 +1090,9 @@ int stats_linkinfoint(Client *client, char *para, int all)
}
#ifdef DEBUGMODE
ircsnprintf(pbuf, sizeof(pbuf), "%ld :%ld", (long)acptr->local->cputime,
(long)(acptr->user && MyConnect(acptr)) ? TStime() - acptr->local->last : 0);
ircsnprintf(pbuf, sizeof(pbuf), "%lld :%lld",
(long long)acptr->local->cputime,
(long long)((acptr->user && MyConnect(acptr)) ? TStime() - acptr->local->last : 0));
#endif
if (ValidatePermissionsForPath("server:info:stats",client,NULL,NULL,NULL))
{
@@ -1104,10 +1105,9 @@ int stats_linkinfoint(Client *client, char *para, int all)
(int)acptr->local->sendM, (int)acptr->local->sendK,
(int)acptr->local->receiveM,
(int)acptr->local->receiveK,
TStime() - acptr->local->firsttime,
(long long)(TStime() - acptr->local->firsttime),
#ifndef DEBUGMODE
(acptr->user && MyConnect(acptr)) ?
TStime() - acptr->local->last : 0);
(long long)((acptr->user && MyConnect(acptr)) ? TStime() - acptr->local->last : 0));
#else
pbuf);
#endif
@@ -1123,10 +1123,9 @@ int stats_linkinfoint(Client *client, char *para, int all)
(int)acptr->local->sendM, (int)acptr->local->sendK,
(int)acptr->local->receiveM,
(int)acptr->local->receiveK,
TStime() - acptr->local->firsttime,
(long long)((TStime() - acptr->local->firsttime)),
#ifndef DEBUGMODE
(acptr->user && MyConnect(acptr)) ?
TStime() - acptr->local->last : 0);
(long long)((acptr->user && MyConnect(acptr)) ? TStime() - acptr->local->last : 0));
#else
pbuf);
#endif
+1
View File
@@ -84,6 +84,7 @@ static int convert_classical_who_request(Client *client, int *parc, char *parv[]
char *whox_md_serialize(ModData *m);
void whox_md_unserialize(char *str, ModData *m);
void whox_md_free(ModData *md);
static void append_format(char *buf, size_t bufsize, size_t *pos, const char *fmt, ...) __attribute__((format(printf,4,5)));
MOD_INIT()
{
+6 -3
View File
@@ -26,10 +26,13 @@
#include "unrealircd.h"
/* Temporarily ignore these for this entire file. FIXME later: */
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
/* Some forward declarions are needed */
void vsendto_one(Client *to, MessageTag *mtags, const char *pattern, va_list vl);
void vsendto_prefix_one(Client *to, Client *from, MessageTag *mtags, const char *pattern, va_list vl);
static int vmakebuf_local_withprefix(char *buf, size_t buflen, Client *from, const char *pattern, va_list vl);
void vsendto_one(Client *to, MessageTag *mtags, const char *pattern, va_list vl) __attribute__((format(printf,3,0)));
void vsendto_prefix_one(Client *to, Client *from, MessageTag *mtags, const char *pattern, va_list vl) __attribute__((format(printf,4,0)));
static int vmakebuf_local_withprefix(char *buf, size_t buflen, Client *from, const char *pattern, va_list vl) __attribute__((format(printf,4,0)));
#define ADD_CRLF(buf, len) { if (len > 510) len = 510; \
buf[len++] = '\r'; buf[len++] = '\n'; buf[len] = '\0'; } while(0)
+6
View File
@@ -61,6 +61,10 @@ extern MOTDLine *find_file(char *, short);
void reread_motdsandrules();
/* Temporarily ignore for this function. FIXME later!!! */
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
/** Send a message upstream if necessary and check if it's for us.
* @param client The sender
* @param mtags Message tags associated with this message
@@ -117,6 +121,8 @@ int hunt_server(Client *client, MessageTag *mtags, char *command, int server, in
return HUNTED_PASS;
}
#pragma GCC diagnostic pop
#ifndef _WIN32
/** Grab operating system name on Windows (outdated) */
char *getosname(void)
+3
View File
@@ -28,6 +28,9 @@
#include "unrealircd.h"
#include "dns.h"
/* Temporarily ignore these for this entire file. FIXME later: */
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
int OpenFiles = 0; /* GLOBAL - number of files currently open */
int readcalls = 0;
+3
View File
@@ -68,6 +68,7 @@
/* Forward declarations - only used for internal (static) functions, of course */
static SecretCache *find_secret_cache(Secret *secr, UnrealDBConfig *cfg);
static void unrealdb_add_to_secret_cache(Secret *secr, UnrealDBConfig *cfg);
static void unrealdb_set_error(UnrealDB *c, UnrealDBError errcode, FORMAT_STRING(const char *pattern), ...) __attribute__((format(printf,3,4)));
UnrealDBError unrealdb_last_error_code;
static char *unrealdb_last_error_string = NULL;
@@ -934,6 +935,7 @@ int unrealdb_read_char(UnrealDB *c, char *t)
/** @} */
#if 0
void fatal_error(FORMAT_STRING(const char *pattern), ...)
{
va_list vl;
@@ -1045,6 +1047,7 @@ void unrealdb_test(void)
fprintf(stderr, "**** TESTING UNENCRYPTED ****\n");
unrealdb_test_speed(NULL);
}
#endif
/** TODO: document and implement
*/