Fix buffer overflow in SCPI_NumberToStr, SCPI_DoubleToStr and SCPI_FloatToStr

Jan Breuer

2016-05-15 91dfcd5a5d2cb20285504d787ee4a6135303891a

Fix buffer overflow in SCPI_NumberToStr, SCPI_DoubleToStr and SCPI_FloatToStr

 examples/common/scpi-def.c

@@ -181,9 +181,9 @@
    const char * data;
    size_t len;

    SCPI_ParamArbitraryBlock(context, &data, &len, FALSE);

    if (SCPI_ParamArbitraryBlock(context, &data, &len, FALSE)) {
    SCPI_ResultArbitraryBlock(context, data, len);
    }

    return SCPI_RES_OK;
}

 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

 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);

    if (result + 1 < len) {
    unit = translateUnitInverse(context->units, value->unit);

    if (unit) {
        strncat(str, " ", len);
        strncat(str, unit, len);
        result += strlen(unit) + 1;
            strncat(str, " ", len - result);
            if (result + 2 < len) {
                strncat(str, unit, len - result - 1);
            }
            result = strlen(str);
        }
    }

    return result;

 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);
}

/**

 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) {