From 91dfcd5a5d2cb20285504d787ee4a6135303891a Mon Sep 17 00:00:00 2001
From: Jan Breuer <jan.breuer@jaybee.cz>
Date: 周日, 15 5月 2016 20:01:44 +0800
Subject: [PATCH] Fix buffer overflow in SCPI_NumberToStr, SCPI_DoubleToStr and SCPI_FloatToStr
---
libscpi/src/units.c | 24 +++++++----
libscpi/inc/scpi/config.h | 8 ++--
libscpi/test/test_parser.c | 53 ++++++++++++++++++++++++++
libscpi/src/utils.c | 6 ++-
examples/common/scpi-def.c | 6 +-
5 files changed, 79 insertions(+), 18 deletions(-)
diff --git a/examples/common/scpi-def.c b/examples/common/scpi-def.c
index d5f71ae..079a846 100644
--- a/examples/common/scpi-def.c
+++ b/examples/common/scpi-def.c
@@ -181,9 +181,9 @@
const char * data;
size_t len;
- SCPI_ParamArbitraryBlock(context, &data, &len, FALSE);
-
- SCPI_ResultArbitraryBlock(context, data, len);
+ if (SCPI_ParamArbitraryBlock(context, &data, &len, FALSE)) {
+ SCPI_ResultArbitraryBlock(context, data, len);
+ }
return SCPI_RES_OK;
}
diff --git a/libscpi/inc/scpi/config.h b/libscpi/inc/scpi/config.h
index 0abb9c2..7473d9d 100644
--- a/libscpi/inc/scpi/config.h
+++ b/libscpi/inc/scpi/config.h
@@ -246,17 +246,17 @@
#endif
#if HAVE_DTOSTRE
-#define SCPIDEFINE_floatToStr(v, s, l) strlen(dtostre((double)(v), (s), 6, DTOSTR_PLUS_SIGN | DTOSTR_ALWAYS_SIGN | DTOSTR_UPPERCASE))
+#define SCPIDEFINE_floatToStr(v, s, l) dtostre((double)(v), (s), 6, DTOSTR_PLUS_SIGN | DTOSTR_ALWAYS_SIGN | DTOSTR_UPPERCASE)
#elif USE_CUSTOM_DTOSTRE
-#define SCPIDEFINE_floatToStr(v, s, l) strlen(SCPI_dtostre((v), (s), (l), 6, 0))
+#define SCPIDEFINE_floatToStr(v, s, l) SCPI_dtostre((v), (s), (l), 6, 0)
#else
#define SCPIDEFINE_floatToStr(v, s, l) snprintf((s), (l), "%g", (v))
#endif
#if HAVE_DTOSTRE
-#define SCPIDEFINE_doubleToStr(v, s, l) strlen(dtostre((v), (s), 15, DTOSTR_PLUS_SIGN | DTOSTR_ALWAYS_SIGN | DTOSTR_UPPERCASE))
+#define SCPIDEFINE_doubleToStr(v, s, l) dtostre((v), (s), 15, DTOSTR_PLUS_SIGN | DTOSTR_ALWAYS_SIGN | DTOSTR_UPPERCASE)
#elif USE_CUSTOM_DTOSTRE
-#define SCPIDEFINE_doubleToStr(v, s, l) strlen(SCPI_dtostre((v), (s), (l), 15, 0))
+#define SCPIDEFINE_doubleToStr(v, s, l) SCPI_dtostre((v), (s), (l), 15, 0)
#else
#define SCPIDEFINE_doubleToStr(v, s, l) snprintf((s), (l), "%.15lg", (v))
#endif
diff --git a/libscpi/src/units.c b/libscpi/src/units.c
index 47c7f9e..9fc2072 100644
--- a/libscpi/src/units.c
+++ b/libscpi/src/units.c
@@ -469,7 +469,7 @@
* @param context
* @param value number value
* @param str target string
- * @param len max length of string
+ * @param len max length of string including null-character termination
* @return number of chars written to string
*/
size_t SCPI_NumberToStr(scpi_t * context, const scpi_choice_def_t * special, scpi_number_t * value, char * str, size_t len) {
@@ -477,28 +477,34 @@
const char * unit;
size_t result;
- if (!value || !str) {
+ if (!value || !str || len==0) {
return 0;
}
if (value->special) {
if (SCPI_ChoiceToName(special, value->tag, &type)) {
strncpy(str, type, len);
- return min(strlen(type), len);
+ result = SCPIDEFINE_strnlen(str, len - 1);
+ str[result] = '\0';
+ return result;
} else {
- str[0] = 0;
+ str[0] = '\0';
return 0;
}
}
result = SCPI_DoubleToStr(value->value, str, len);
- unit = translateUnitInverse(context->units, value->unit);
+ if (result + 1 < len) {
+ unit = translateUnitInverse(context->units, value->unit);
- if (unit) {
- strncat(str, " ", len);
- strncat(str, unit, len);
- result += strlen(unit) + 1;
+ if (unit) {
+ strncat(str, " ", len - result);
+ if (result + 2 < len) {
+ strncat(str, unit, len - result - 1);
+ }
+ result = strlen(str);
+ }
}
return result;
diff --git a/libscpi/src/utils.c b/libscpi/src/utils.c
index cb34cc7..6fa5ad7 100644
--- a/libscpi/src/utils.c
+++ b/libscpi/src/utils.c
@@ -249,7 +249,8 @@
* @return number of bytes written to str (without '\0')
*/
size_t SCPI_FloatToStr(float val, char * str, size_t len) {
- return SCPIDEFINE_floatToStr(val, str, len);
+ SCPIDEFINE_floatToStr(val, str, len);
+ return strlen(str);
}
/**
@@ -260,7 +261,8 @@
* @return number of bytes written to str (without '\0')
*/
size_t SCPI_DoubleToStr(double val, char * str, size_t len) {
- return SCPIDEFINE_doubleToStr(val, str, len);
+ SCPIDEFINE_doubleToStr(val, str, len);
+ return strlen(str);
}
/**
diff --git a/libscpi/test/test_parser.c b/libscpi/test/test_parser.c
index f2ff12f..ca313aa 100644
--- a/libscpi/test/test_parser.c
+++ b/libscpi/test/test_parser.c
@@ -1424,9 +1424,62 @@
CU_ASSERT_EQUAL(res_len, strlen(expected_result));\
} while(0)
+#define TEST_SCPI_NumberToStr_limited(_special, _value, _unit, expected_result, limit) do {\
+ scpi_number_t number;\
+ number.base = 10;\
+ number.special = (_special);\
+ number.unit = (_unit);\
+ if (number.special) { number.tag = (int)(_value); } else { number.value = (_value); }\
+ char buffer[100];\
+ memset(buffer, 0xaa, 100);\
+ size_t res_len;\
+ res_len = SCPI_NumberToStr(&scpi_context, scpi_special_numbers_def, &number, buffer, limit);\
+ size_t expected_len = strnlen(expected_result, limit - 1);\
+ CU_ASSERT_NSTRING_EQUAL(buffer, expected_result, expected_len);\
+ CU_ASSERT_EQUAL(buffer[expected_len], 0);\
+ CU_ASSERT_EQUAL((unsigned char)buffer[limit], 0xaa);\
+ CU_ASSERT_EQUAL(res_len, expected_len);\
+} while(0)
+
TEST_SCPI_NumberToStr(FALSE, 10.5, SCPI_UNIT_NONE, "10.5");
TEST_SCPI_NumberToStr(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V");
TEST_SCPI_NumberToStr(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault");
+
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_NONE, "10.5", 1);
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V", 1);
+ TEST_SCPI_NumberToStr_limited(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault", 1);
+
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_NONE, "10.5", 2);
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V", 2);
+ TEST_SCPI_NumberToStr_limited(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault", 2);
+
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_NONE, "10.5", 3);
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V", 3);
+ TEST_SCPI_NumberToStr_limited(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault", 3);
+
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_NONE, "10.5", 4);
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V", 4);
+ TEST_SCPI_NumberToStr_limited(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault", 4);
+
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_NONE, "10.5", 5);
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V", 5);
+ TEST_SCPI_NumberToStr_limited(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault", 5);
+
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_NONE, "10.5", 6);
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V", 6);
+ TEST_SCPI_NumberToStr_limited(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault", 6);
+
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_NONE, "10.5", 7);
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V", 7);
+ TEST_SCPI_NumberToStr_limited(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault", 7);
+
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_NONE, "10.5", 8);
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V", 8);
+ TEST_SCPI_NumberToStr_limited(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault", 8);
+
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_NONE, "10.5", 9);
+ TEST_SCPI_NumberToStr_limited(FALSE, 10.5, SCPI_UNIT_VOLT, "10.5 V", 9);
+ TEST_SCPI_NumberToStr_limited(TRUE, SCPI_NUM_DEF, SCPI_UNIT_NONE, "DEFault", 9);
}
static void testErrorQueue(void) {
--
Gitblit v1.9.1