From f7d2250b85a6ff1f83ce59d96b9c4bdf3bc8b859 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sun, 8 Nov 2020 07:41:35 +0100 Subject: [PATCH 1/3] drivers/netxml-ups.c: rearrange sscanf() of inputs to avoid wrong int values --- drivers/netxml-ups.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/netxml-ups.c b/drivers/netxml-ups.c index 9ee3830996..3dbe3d5278 100644 --- a/drivers/netxml-ups.c +++ b/drivers/netxml-ups.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -773,15 +774,34 @@ static int netxml_alarm_subscribe(const char *page) /* due to different formats used by the various NMCs, we need to\ break up the reply in lines and parse each one separately */ for (s = strtok(resp_buf, "\r\n"); s != NULL; s = strtok(NULL, "\r\n")) { + long long int tmp_port = -1, tmp_secret = -1; upsdebugx(2, "%s: parsing %s", __func__, s); - if (!strncasecmp(s, "", 6) && (sscanf(s+6, "%u", &port) != 1)) { + if (!strncasecmp(s, "", 6) && (sscanf(s+6, "%lli", &tmp_port) != 1)) { return NE_RETRY; } - if (!strncasecmp(s, "", 8) && (sscanf(s+8, "%u", &secret) != 1)) { + /* FIXME? Does a port==0 make sense here? Or should the test below be for port<1? + * Legacy code until a fix here used sscanf() above to get a '%u' value... + */ + if (tmp_port < 0 || tmp_port > UINT_MAX) { + upsdebugx(2, "%s: parsing initial subcription failed, bad port value", __func__); return NE_RETRY; } + + if (!strncasecmp(s, "", 8) && (sscanf(s+8, "%lli", &tmp_secret) != 1)) { + return NE_RETRY; + } + + if (tmp_secret < 0 || tmp_secret > UINT_MAX) { + upsdebugx(2, "%s: parsing initial subcription failed, bad secret value", __func__); + return NE_RETRY; + } + + /* Range of valid values constrained above */ + port = tmp_port; + secret = tmp_secret; + } if ((port == -1) || (secret == -1)) { From 6c09bd1d84595e0153e8d6f427f291ff12101090 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 6 Nov 2020 20:33:01 +0100 Subject: [PATCH 2/3] drivers/nutdrv_qx.c: upsdrv_initups(): constrain langid_fix scanned values to fit into signed int --- drivers/nutdrv_qx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/nutdrv_qx.c b/drivers/nutdrv_qx.c index bb526608c7..42302baf69 100644 --- a/drivers/nutdrv_qx.c +++ b/drivers/nutdrv_qx.c @@ -1944,9 +1944,11 @@ void upsdrv_initups(void) /* Check for language ID workaround (#1) */ if (getval("langid_fix")) { /* Skip "0x" prefix and set back to hexadecimal */ - if (sscanf(getval("langid_fix") + 2, "%x", &langid_fix) != 1) { + unsigned int u_langid_fix; + if ( (sscanf(getval("langid_fix") + 2, "%x", &u_langid_fix) != 1) || (u_langid_fix > INT_MAX)) { upslogx(LOG_NOTICE, "Error enabling language ID workaround"); } else { + langid_fix = u_langid_fix; upsdebugx(2, "Language ID workaround enabled (using '0x%x')", langid_fix); } } From f349ed8abe1c092ed048e93ed26c21f4a08abfe5 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sun, 8 Nov 2020 07:44:43 +0100 Subject: [PATCH 3/3] drivers/blazer_usb.c: upsdrv_initups(): constrain langid_fix scanned values to fit into signed int --- drivers/blazer_usb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/blazer_usb.c b/drivers/blazer_usb.c index 68d7b8674d..2aa301a757 100644 --- a/drivers/blazer_usb.c +++ b/drivers/blazer_usb.c @@ -536,10 +536,12 @@ void upsdrv_initups(void) /* check for language ID workaround (#1) */ if (getval("langid_fix")) { /* skip "0x" prefix and set back to hexadecimal */ - if (sscanf(getval("langid_fix") + 2, "%x", &langid_fix) != 1) { + unsigned int u_langid_fix; + if ( (sscanf(getval("langid_fix") + 2, "%x", &u_langid_fix) != 1) || (u_langid_fix > INT_MAX) ) { upslogx(LOG_NOTICE, "Error enabling language ID workaround"); } else { + langid_fix = u_langid_fix; upsdebugx(2, "language ID workaround enabled (using '0x%x')", langid_fix); } }