Skip to content

Commit

Permalink
Harden NUT work with strings by switching to snprintf_dynamic() inste…
Browse files Browse the repository at this point in the history
…ad of hushing potential flaws with macros [networkupstools#2450]

Found by pragmas to clean up, with

    :; git grep -En 'Wformat-(sec|nonlit)'

Signed-off-by: Jim Klimov <[email protected]>
  • Loading branch information
jimklimov committed Jun 2, 2024
1 parent 5a3986e commit 6946774
Show file tree
Hide file tree
Showing 24 changed files with 179 additions and 966 deletions.
54 changes: 15 additions & 39 deletions clients/upsclient.c
Original file line number Diff line number Diff line change
Expand Up @@ -554,16 +554,6 @@ const char *upscli_strerror(UPSCONN_t *ups)
char sslbuf[UPSCLI_ERRBUF_LEN];
#endif

#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_SECURITY
#pragma GCC diagnostic ignored "-Wformat-security"
#endif

if (!ups) {
return upscli_errlist[UPSCLI_ERR_INVALIDARG].str;
}
Expand All @@ -582,23 +572,26 @@ const char *upscli_strerror(UPSCONN_t *ups)
return upscli_errlist[ups->upserror].str;

case 1: /* add message from system's strerror */
snprintf(ups->errbuf, UPSCLI_ERRBUF_LEN,
snprintf_dynamic(
ups->errbuf, UPSCLI_ERRBUF_LEN,
upscli_errlist[ups->upserror].str,
strerror(ups->syserrno));
"%s", strerror(ups->syserrno));
return ups->errbuf;

case 2: /* SSL error */
#ifdef WITH_OPENSSL
err = ERR_get_error();
if (err) {
ERR_error_string(err, sslbuf);
snprintf(ups->errbuf, UPSCLI_ERRBUF_LEN,
snprintf_dynamic(
ups->errbuf, UPSCLI_ERRBUF_LEN,
upscli_errlist[ups->upserror].str,
sslbuf);
"%s", sslbuf);
} else {
snprintf(ups->errbuf, UPSCLI_ERRBUF_LEN,
snprintf_dynamic(
ups->errbuf, UPSCLI_ERRBUF_LEN,
upscli_errlist[ups->upserror].str,
"peer disconnected");
"%s", "peer disconnected");
}
#elif defined(WITH_NSS) /* WITH_OPENSSL */
if (PR_GetErrorTextLength() < UPSCLI_ERRBUF_LEN) {
Expand All @@ -615,16 +608,13 @@ const char *upscli_strerror(UPSCONN_t *ups)
return ups->errbuf;

case 3: /* parsing (parseconf) error */
snprintf(ups->errbuf, UPSCLI_ERRBUF_LEN,
snprintf_dynamic(
ups->errbuf, UPSCLI_ERRBUF_LEN,
upscli_errlist[ups->upserror].str,
ups->pc_ctx.errmsg);
"%s", ups->pc_ctx.errmsg);
return ups->errbuf;
}

#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif

/* fallthrough */

snprintf(ups->errbuf, UPSCLI_ERRBUF_LEN, "Unknown error flag %d",
Expand Down Expand Up @@ -1316,23 +1306,9 @@ static void build_cmd(char *buf, size_t bufsize, const char *cmdname,
format = " %s";
}

/* snprintfcat would tie us to common */

len = strlen(buf);
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_SECURITY
#pragma GCC diagnostic ignored "-Wformat-security"
#endif
snprintf(buf + len, bufsize - len, format,
pconf_encode(arg[i], enc, sizeof(enc)));
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif
snprintfcat_dynamic(
buf, bufsize, format,
"%s", pconf_encode(arg[i], enc, sizeof(enc)));
}

len = strlen(buf);
Expand Down
18 changes: 4 additions & 14 deletions clients/upsimage.c
Original file line number Diff line number Diff line change
Expand Up @@ -275,20 +275,10 @@ static void drawbar(
gdImageFilledRectangle(im, 25, bar_y, width - 25, scale_height,
bar_color);

/* stick the text version of the value at the bottom center */
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_SECURITY
#pragma GCC diagnostic ignored "-Wformat-security"
#endif
snprintf(text, sizeof(text), format, value);
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif
/* stick the text version of the value at the bottom center
* expected format is one of imgvar[] entries for "double value"
*/
snprintf_dynamic(text, sizeof(text), format, "%f", value);
gdImageString(im, gdFontMediumBold,
(width - (int)(strlen(text))*gdFontMediumBold->w)/2,
height - gdFontMediumBold->h,
Expand Down
17 changes: 4 additions & 13 deletions clients/upsmon.c
Original file line number Diff line number Diff line change
Expand Up @@ -378,21 +378,12 @@ static void do_notify(const utype_t *ups, int ntype)
if (notifylist[i].type == ntype) {
upsdebugx(2, "%s: ntype 0x%04x (%s)", __func__, ntype,
notifylist[i].name);
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_SECURITY
#pragma GCC diagnostic ignored "-Wformat-security"
#endif
snprintf(msg, sizeof(msg),

snprintf_dynamic(msg, sizeof(msg),
notifylist[i].msg ? notifylist[i].msg : notifylist[i].stockmsg,
"%s",
ups ? ups->sys : "");
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif

notify(msg, notifylist[i].flags, notifylist[i].name,
upsname);

Expand Down
41 changes: 8 additions & 33 deletions drivers/apc_modbus.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
#include <modbus.h>

#define DRIVER_NAME "NUT APC Modbus driver"
#define DRIVER_VERSION "0.10"
#define DRIVER_VERSION "0.11"

#if defined NUT_MODBUS_HAS_USB

Expand Down Expand Up @@ -389,16 +389,8 @@ static int _apc_modbus_double_to_nut(const apc_modbus_value_t *value, char *outp
if (value->format != NULL)
format = value->format;

#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
res = snprintf(output, output_len, format, double_value);
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif
res = snprintf_dynamic(output, output_len, format, "%f", double_value);

if (res < 0 || (size_t)res >= output_len) {
return 0;
}
Expand Down Expand Up @@ -432,16 +424,8 @@ static int _apc_modbus_power_to_nut(const apc_modbus_value_t *value, char *outpu
if (value->format != NULL)
format = value->format;

#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
res = snprintf(output, output_len, format, double_value);
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif
res = snprintf_dynamic(output, output_len, format, "%f", double_value);

if (res < 0 || (size_t)res >= output_len) {
return 0;
}
Expand Down Expand Up @@ -1059,27 +1043,18 @@ static int _apc_modbus_update_value(apc_modbus_register_t *regs_info, const uint
}
dstate_setinfo(regs_info->nut_variable_name, "%s", nutvbuf);
} else {
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
assert(regs_info->value_type <= apc_modbus_value_types_max);
switch (regs_info->value_type) {
case APC_VT_STRING:
dstate_setinfo(regs_info->nut_variable_name, regs_info->value_format, value.data.string_value);
dstate_setinfo_dynamic(regs_info->nut_variable_name, regs_info->value_format, "%s", value.data.string_value);
break;
case APC_VT_INT:
dstate_setinfo(regs_info->nut_variable_name, regs_info->value_format, value.data.int_value);
dstate_setinfo_dynamic(regs_info->nut_variable_name, regs_info->value_format, "%" PRIi64, value.data.int_value);
break;
case APC_VT_UINT:
dstate_setinfo(regs_info->nut_variable_name, regs_info->value_format, value.data.uint_value);
dstate_setinfo_dynamic(regs_info->nut_variable_name, regs_info->value_format, "%" PRIu64, value.data.uint_value);
break;
}
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif
}

dstate_flags = 0;
Expand Down
18 changes: 3 additions & 15 deletions drivers/apcupsd-ups.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ typedef struct pollfd {
#include "nut_stdint.h"

#define DRIVER_NAME "apcupsd network client UPS driver"
#define DRIVER_VERSION "0.72"
#define DRIVER_VERSION "0.73"

#define POLL_INTERVAL_MIN 10

Expand Down Expand Up @@ -163,22 +163,10 @@ static void process(char *item,char *data)
}
else
{
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_SECURITY
#pragma GCC diagnostic ignored "-Wformat-security"
#endif
/* default_value acts as a format string in this case */
dstate_setinfo(nut_data[i].info_type,
dstate_setinfo_dynamic(nut_data[i].info_type,
nut_data[i].default_value,
atof(data)*nut_data[i].info_len);
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif
"%f", atof(data)*nut_data[i].info_len);
}
break;
}
Expand Down
16 changes: 2 additions & 14 deletions drivers/bcmxcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ TODO List:
#include "bcmxcp.h"

#define DRIVER_NAME "BCMXCP UPS driver"
#define DRIVER_VERSION "0.34"
#define DRIVER_VERSION "0.35"

#define MAX_NUT_NAME_LENGTH 128
#define NUT_OUTLET_POSITION 7
Expand Down Expand Up @@ -897,19 +897,7 @@ void decode_meter_map_entry(const unsigned char *entry, const unsigned char form
fValue = get_float(entry);
/* Format is packed BCD */
snprintf(sFormat, 31, "%%%d.%df", ((format & 0xf0) >> 4), (format & 0x0f));
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_SECURITY
#pragma GCC diagnostic ignored "-Wformat-security"
#endif
snprintf(value, 127, sFormat, fValue);
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif
snprintf_dynamic(value, 127, sFormat, "%f", fValue);
}
else if (format == 0xe2) {
/* Seconds */
Expand Down
16 changes: 2 additions & 14 deletions drivers/bestfortress.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
#endif

#define DRIVER_NAME "Best Fortress UPS driver"
#define DRIVER_VERSION "0.09"
#define DRIVER_VERSION "0.10"

/* driver description structure */
upsdrv_info_t upsdrv_info = {
Expand Down Expand Up @@ -185,19 +185,7 @@ static inline void setinfo_float (const char *key, const char * fmt, const char
strncpy (buf, s, len);
buf[len] = 0;

#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_SECURITY
#pragma GCC diagnostic ignored "-Wformat-security"
#endif
dstate_setinfo (key, fmt, factor * (double)(atoi (buf)));
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif
dstate_setinfo_dynamic (key, fmt, "%f", factor * (double)(atoi (buf)));
}

static int upssend(const char *fmt,...) {
Expand Down
32 changes: 2 additions & 30 deletions drivers/blazer.c
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,6 @@ static int blazer_status(const char *cmd)
}

for (i = 0, val = strtok_r(buf+1, " ", &last); status[i].var; i++, val = strtok_r(NULL, " \r\n", &last)) {

if (!val) {
upsdebugx(2, "%s: parsing failed", __func__);
return -1;
Expand All @@ -215,20 +214,7 @@ static int blazer_status(const char *cmd)
continue;
}

#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_SECURITY
#pragma GCC diagnostic ignored "-Wformat-security"
#endif
dstate_setinfo(status[i].var, status[i].fmt, status[i].conv(val, NULL));
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif

dstate_setinfo_dynamic(status[i].var, status[i].fmt, "%f", status[i].conv(val, NULL));
}

if (!val) {
Expand Down Expand Up @@ -344,7 +330,6 @@ static int blazer_rating(const char *cmd)
}

for (i = 0, val = strtok_r(buf+1, " ", &last); rating[i].var; i++, val = strtok_r(NULL, " \r\n", &last)) {

if (!val) {
upsdebugx(2, "%s: parsing failed", __func__);
return -1;
Expand All @@ -355,20 +340,7 @@ static int blazer_rating(const char *cmd)
continue;
}

#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_SECURITY
#pragma GCC diagnostic ignored "-Wformat-security"
#endif
dstate_setinfo(rating[i].var, rating[i].fmt, rating[i].conv(val, NULL));
#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
#pragma GCC diagnostic pop
#endif

dstate_setinfo_dynamic(rating[i].var, rating[i].fmt, "%f", rating[i].conv(val, NULL));
}

return 0;
Expand Down
2 changes: 1 addition & 1 deletion drivers/blazer_ser.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
#include "blazer.h"

#define DRIVER_NAME "Megatec/Q1 protocol serial driver"
#define DRIVER_VERSION "1.62"
#define DRIVER_VERSION "1.63"

/* driver description structure */
upsdrv_info_t upsdrv_info = {
Expand Down
2 changes: 1 addition & 1 deletion drivers/blazer_usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#endif

#define DRIVER_NAME "Megatec/Q1 protocol USB driver"
#define DRIVER_VERSION "0.19"
#define DRIVER_VERSION "0.20"

/* driver description structure */
upsdrv_info_t upsdrv_info = {
Expand Down
Loading

0 comments on commit 6946774

Please sign in to comment.