Skip to content

Commit

Permalink
Fix buffer overflow in SCPI_NumberToStr, SCPI_DoubleToStr and SCPI_Fl…
Browse files Browse the repository at this point in the history
…oatToStr
  • Loading branch information
j123b567 committed May 15, 2016
1 parent 71c6c18 commit 91dfcd5
Showing 5 changed files with 79 additions and 18 deletions.
6 changes: 3 additions & 3 deletions examples/common/scpi-def.c
Original file line number Diff line number Diff line change
@@ -181,9 +181,9 @@ static scpi_result_t TEST_ArbQ(scpi_t * context) {
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;
}
8 changes: 4 additions & 4 deletions libscpi/inc/scpi/config.h
Original file line number Diff line number Diff line change
@@ -246,17 +246,17 @@ extern "C" {
#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
24 changes: 15 additions & 9 deletions libscpi/src/units.c
Original file line number Diff line number Diff line change
@@ -469,36 +469,42 @@ scpi_bool_t SCPI_ParamNumber(scpi_t * context, const scpi_choice_def_t * special
* @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) {
const char * type;
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;
6 changes: 4 additions & 2 deletions libscpi/src/utils.c
Original file line number Diff line number Diff line change
@@ -249,7 +249,8 @@ size_t SCPI_UInt64ToStrBase(uint64_t val, char * str, size_t len, int8_t base) {
* @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 @@ size_t SCPI_FloatToStr(float val, char * str, size_t len) {
* @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);
}

/**
53 changes: 53 additions & 0 deletions libscpi/test/test_parser.c
Original file line number Diff line number Diff line change
@@ -1424,9 +1424,62 @@ static void testNumberToStr(void) {
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) {

0 comments on commit 91dfcd5

Please sign in to comment.