Bugfix/service request refactor size check (#553)

* refactor service requests from service header

* add APDU size checking and length feature

* add unit tests to check for length when passing NULL buffer

---------

Co-authored-by: Steve Karg <skarg@users.sourceforge.net>
This commit is contained in:
Steve Karg
2024-01-05 08:59:45 -06:00
committed by GitHub
parent 5ca14e5320
commit bb081d28da
39 changed files with 2614 additions and 1514 deletions
+6 -4
View File
@@ -165,13 +165,14 @@ static void testCOVNotifyData(BACNET_COV_DATA *data, BACNET_COV_DATA *test_data)
static void testUCOVNotifyData(BACNET_COV_DATA *data)
{
uint8_t apdu[480] = { 0 };
int len = 0;
int apdu_len = 0;
int len = 0, null_len = 0, apdu_len = 0;
BACNET_COV_DATA test_data = { 0 };
BACNET_PROPERTY_VALUE value_list[5] = { { 0 } };
null_len = ucov_notify_encode_apdu(NULL, sizeof(apdu), data);
len = ucov_notify_encode_apdu(&apdu[0], sizeof(apdu), data);
zassert_true(len > 0, NULL);
zassert_equal(len, null_len, NULL);
apdu_len = len;
cov_data_value_list_link(
@@ -184,14 +185,15 @@ static void testUCOVNotifyData(BACNET_COV_DATA *data)
static void testCCOVNotifyData(uint8_t invoke_id, BACNET_COV_DATA *data)
{
uint8_t apdu[480] = { 0 };
int len = 0;
int apdu_len = 0;
int len = 0, null_len = 0, apdu_len = 0;
BACNET_COV_DATA test_data = { 0 };
BACNET_PROPERTY_VALUE value_list[2] = { { 0 } };
uint8_t test_invoke_id = 0;
null_len = ccov_notify_encode_apdu(NULL, sizeof(apdu), invoke_id, data);
len = ccov_notify_encode_apdu(&apdu[0], sizeof(apdu), invoke_id, data);
zassert_not_equal(len, 0, NULL);
zassert_equal(len, null_len, NULL);
apdu_len = len;
cov_data_value_list_link(&test_data, &value_list[0], 2);
+41 -29
View File
@@ -20,60 +20,74 @@
* @brief Test
*/
static int dcc_decode_apdu(uint8_t *apdu,
unsigned apdu_len,
unsigned apdu_size,
uint8_t *invoke_id,
uint16_t *timeDuration,
BACNET_COMMUNICATION_ENABLE_DISABLE *enable_disable,
BACNET_CHARACTER_STRING *password)
{
int len = 0;
unsigned offset = 0;
unsigned apdu_len = 0;
if (!apdu)
return -1;
/* optional checking - most likely was already done prior to this call */
if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST)
return -1;
/* apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
*invoke_id = apdu[2]; /* invoke id - filled in by net layer */
if (apdu[3] != SERVICE_CONFIRMED_DEVICE_COMMUNICATION_CONTROL)
return -1;
offset = 4;
if (apdu_len > offset) {
len = dcc_decode_service_request(&apdu[offset], apdu_len - offset,
if (!apdu) {
return BACNET_STATUS_ERROR;
}
if (apdu_size > 4) {
if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
return BACNET_STATUS_ERROR;
}
/* apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
*invoke_id = apdu[2]; /* invoke id - filled in by net layer */
if (apdu[3] != SERVICE_CONFIRMED_DEVICE_COMMUNICATION_CONTROL) {
return BACNET_STATUS_ERROR;
}
apdu_len = 4;
} else {
return BACNET_STATUS_ERROR;
}
if (apdu_size > apdu_len) {
len = dcc_decode_service_request(&apdu[apdu_len], apdu_size - apdu_len,
timeDuration, enable_disable, password);
if (len > 0) {
apdu_len += len;
} else {
apdu_len = len;
}
}
return len;
return apdu_len;
}
static void test_DeviceCommunicationControlData(
uint8_t invoke_id,
static void test_DeviceCommunicationControlData(uint8_t invoke_id,
uint16_t timeDuration,
BACNET_COMMUNICATION_ENABLE_DISABLE enable_disable,
BACNET_CHARACTER_STRING *password)
{
uint8_t apdu[480] = { 0 };
int len = 0;
int apdu_len = 0;
int apdu_size = 0, null_len = 0, test_len = 0;
uint8_t test_invoke_id = 0;
uint16_t test_timeDuration = 0;
BACNET_COMMUNICATION_ENABLE_DISABLE test_enable_disable;
BACNET_CHARACTER_STRING test_password;
len = dcc_encode_apdu(
null_len = dcc_encode_apdu(
NULL, invoke_id, timeDuration, enable_disable, password);
apdu_size = dcc_encode_apdu(
&apdu[0], invoke_id, timeDuration, enable_disable, password);
zassert_not_equal(len, 0, NULL);
apdu_len = len;
zassert_equal(apdu_size, null_len, NULL);
zassert_not_equal(apdu_size, 0, NULL);
len = dcc_decode_apdu(&apdu[0], apdu_len, &test_invoke_id,
test_len = dcc_decode_apdu(&apdu[0], apdu_size, &test_invoke_id,
&test_timeDuration, &test_enable_disable, &test_password);
zassert_not_equal(len, -1, NULL);
zassert_not_equal(test_len, -1, NULL);
zassert_equal(test_invoke_id, invoke_id, NULL);
zassert_equal(test_timeDuration, timeDuration, NULL);
zassert_equal(test_enable_disable, enable_disable, NULL);
zassert_true(characterstring_same(&test_password, password), NULL);
test_len = dcc_decode_apdu(apdu, 4, &test_invoke_id,
&test_timeDuration, &test_enable_disable, &test_password);
zassert_true(test_len < 0, "apdu_size=%d test_len=%d",
apdu_size, test_len);
}
#if defined(CONFIG_ZTEST_NEW_API)
@@ -149,16 +163,14 @@ static void test_DeviceCommunicationControlMalformedData(void)
* @}
*/
#if defined(CONFIG_ZTEST_NEW_API)
ZTEST_SUITE(dcc_tests, NULL, NULL, NULL, NULL, NULL);
#else
void test_main(void)
{
ztest_test_suite(dcc_tests,
ztest_unit_test(test_DeviceCommunicationControl),
ztest_unit_test(test_DeviceCommunicationControlMalformedData)
);
ztest_unit_test(test_DeviceCommunicationControl),
ztest_unit_test(test_DeviceCommunicationControlMalformedData));
ztest_run_test_suite(dcc_tests);
}
File diff suppressed because it is too large Load Diff
+36 -24
View File
@@ -20,30 +20,39 @@
* @brief Test
*/
static int getevent_decode_apdu(uint8_t *apdu,
unsigned apdu_len,
unsigned apdu_size,
uint8_t *invoke_id,
BACNET_OBJECT_ID *lastReceivedObjectIdentifier)
{
int len = 0;
unsigned offset = 0;
unsigned apdu_len = 0;
if (!apdu)
return -1;
/* optional checking - most likely was already done prior to this call */
if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST)
return -1;
/* apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
*invoke_id = apdu[2]; /* invoke id - filled in by net layer */
if (apdu[3] != SERVICE_CONFIRMED_GET_EVENT_INFORMATION)
return -1;
offset = 4;
if (apdu_len > offset) {
len = getevent_decode_service_request(
&apdu[offset], apdu_len - offset, lastReceivedObjectIdentifier);
if (!apdu) {
return BACNET_STATUS_ERROR;
}
if (apdu_size > 4) {
if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
return BACNET_STATUS_ERROR;
}
/* apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
*invoke_id = apdu[2]; /* invoke id - filled in by net layer */
if (apdu[3] != SERVICE_CONFIRMED_GET_EVENT_INFORMATION) {
return BACNET_STATUS_ERROR;
}
apdu_len = 4;
}
return len;
if (apdu_size > apdu_len) {
len = getevent_decode_service_request(
&apdu[apdu_len], apdu_size - apdu_len, lastReceivedObjectIdentifier);
if (len > 0) {
apdu_len += len;
} else {
apdu_len = len;
}
}
return apdu_len;
}
static int getevent_ack_decode_apdu(uint8_t *apdu,
@@ -150,8 +159,7 @@ static void testGetEventInformation(void)
#endif
{
uint8_t apdu[480] = { 0 };
int len = 0;
int apdu_len = 0;
int apdu_len, test_len, null_len;
uint8_t invoke_id = 128;
uint8_t test_invoke_id = 0;
BACNET_OBJECT_ID lastReceivedObjectIdentifier;
@@ -159,14 +167,18 @@ static void testGetEventInformation(void)
lastReceivedObjectIdentifier.type = OBJECT_BINARY_INPUT;
lastReceivedObjectIdentifier.instance = 12345;
len = getevent_encode_apdu(
null_len = getevent_encode_apdu(
NULL, invoke_id, &lastReceivedObjectIdentifier);
apdu_len = getevent_encode_apdu(
&apdu[0], invoke_id, &lastReceivedObjectIdentifier);
zassert_not_equal(len, 0, NULL);
apdu_len = len;
zassert_equal(apdu_len, null_len, NULL);
zassert_not_equal(apdu_len, 0, NULL);
len = getevent_decode_apdu(&apdu[0], apdu_len, &test_invoke_id,
test_len = getevent_decode_apdu(&apdu[0], apdu_len, &test_invoke_id,
&test_lastReceivedObjectIdentifier);
zassert_not_equal(len, -1, NULL);
zassert_equal(
apdu_len, test_len, "apdu_len=%d test_len=%d", apdu_len, test_len);
zassert_not_equal(test_len, -1, NULL);
zassert_equal(test_invoke_id, invoke_id, NULL);
zassert_equal(
test_lastReceivedObjectIdentifier.type,
+8
View File
@@ -27,9 +27,17 @@ static void test_ListElement(void)
list_element.application_data = NULL;
list_element.application_data_len = 0;
/* NULL test - number of bytes which would have been written to the APDU */
null_len = list_element_encode_service_request(NULL, &list_element);
apdu_len = list_element_encode_service_request(apdu, &list_element);
zassert_equal(apdu_len, null_len, NULL);
apdu_len =
list_element_service_request_encode(apdu, null_len, &list_element);
zassert_equal(apdu_len, null_len, NULL);
/* negative test - too short */
null_len = list_element_service_request_encode(NULL, 0, &list_element);
zassert_equal(0, null_len, NULL);
/* decoder test */
test_len =
list_element_decode_service_request(apdu, apdu_len, &test_list_element);
zassert_equal(apdu_len, test_len, NULL);
+20 -18
View File
@@ -25,13 +25,12 @@ ZTEST(lso_tests, testLSO)
static void testLSO(void)
#endif
{
uint8_t apdu[1000];
int len;
uint8_t apdu[1000] = { 0 };
uint8_t invoke_id = 100;
int apdu_len = 0, null_len = 0, test_len = 0;
BACNET_LSO_DATA data;
BACNET_LSO_DATA rxdata;
memset(&rxdata, 0, sizeof(rxdata));
BACNET_LSO_DATA data = { 0 };
BACNET_LSO_DATA test_data = { 0 };
characterstring_init_ansi(&data.requestingSrc, "foobar");
data.operation = LIFE_SAFETY_OP_RESET;
@@ -39,19 +38,22 @@ static void testLSO(void)
data.use_target = true;
data.targetObject.instance = 0x1000;
data.targetObject.type = OBJECT_BINARY_INPUT;
len = lso_encode_apdu(apdu, 100, &data);
lso_decode_service_request(&apdu[4], len, &rxdata);
zassert_equal(data.operation, rxdata.operation, NULL);
zassert_equal(data.processId, rxdata.processId, NULL);
zassert_equal(data.use_target, rxdata.use_target, NULL);
zassert_equal(data.targetObject.instance, rxdata.targetObject.instance, NULL);
zassert_equal(data.targetObject.type, rxdata.targetObject.type, NULL);
/* encode/decode */
null_len = lso_encode_apdu(NULL, invoke_id, &data);
apdu_len = lso_encode_apdu(apdu, invoke_id, &data);
zassert_equal(
memcmp(data.requestingSrc.value, rxdata.requestingSrc.value,
rxdata.requestingSrc.length), 0, NULL);
apdu_len, null_len, "apdu_len=%d null_len=%d", apdu_len, null_len);
test_len = lso_decode_service_request(&apdu[4], apdu_len, &test_data);
zassert_true(test_len > 0, "test_len=%d", test_len);
/* check the values decoded */
zassert_equal(data.operation, test_data.operation, NULL);
zassert_equal(data.processId, test_data.processId, NULL);
zassert_equal(data.use_target, test_data.use_target, NULL);
zassert_equal(data.targetObject.instance, test_data.targetObject.instance, NULL);
zassert_equal(data.targetObject.type, test_data.targetObject.type, NULL);
zassert_equal(
memcmp(data.requestingSrc.value, test_data.requestingSrc.value,
test_data.requestingSrc.length), 0, NULL);
}
/**
* @}
+4 -4
View File
@@ -30,21 +30,21 @@ static int rd_decode_apdu(uint8_t *apdu,
int apdu_len = 0;
if (!apdu) {
return -1;
return BACNET_STATUS_ERROR;
}
if (apdu_size <= 4) {
return -1;
return BACNET_STATUS_ERROR;
}
/* optional checking - most likely was already done prior to this call */
if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
return -1;
return BACNET_STATUS_ERROR;
}
/* apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
if (invoke_id) {
*invoke_id = apdu[2]; /* invoke id - filled in by net layer */
}
if (apdu[3] != SERVICE_CONFIRMED_REINITIALIZE_DEVICE) {
return -1;
return BACNET_STATUS_ERROR;
}
apdu_len = 4;
if (apdu_len < apdu_size) {
+91 -49
View File
@@ -21,55 +21,77 @@
* @brief Test
*/
static int rp_decode_apdu(uint8_t *apdu,
unsigned apdu_len,
unsigned apdu_size,
uint8_t *invoke_id,
BACNET_READ_PROPERTY_DATA *rpdata)
{
int len = 0;
unsigned offset = 0;
unsigned apdu_len = 0;
if (!apdu)
return -1;
if (!apdu) {
return BACNET_STATUS_ERROR;
}
if (apdu_size <= 4) {
return BACNET_STATUS_ERROR;
}
/* optional checking - most likely was already done prior to this call */
if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST)
return -1;
if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
return BACNET_STATUS_ERROR;
}
/* apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
*invoke_id = apdu[2]; /* invoke id - filled in by net layer */
if (apdu[3] != SERVICE_CONFIRMED_READ_PROPERTY)
return -1;
offset = 4;
if (apdu[3] != SERVICE_CONFIRMED_READ_PROPERTY) {
return BACNET_STATUS_ERROR;
}
apdu_len = 4;
if (apdu_len > offset) {
len =
rp_decode_service_request(&apdu[offset], apdu_len - offset, rpdata);
if (apdu_size > apdu_len) {
len = rp_decode_service_request(
&apdu[apdu_len], apdu_size - apdu_len, rpdata);
if (len > 0) {
apdu_len += len;
} else {
apdu_len = len;
}
}
return len;
return apdu_len;
}
static int rp_ack_decode_apdu(uint8_t *apdu,
int apdu_len, /* total length of the apdu */
int apdu_size,
uint8_t *invoke_id,
BACNET_READ_PROPERTY_DATA *rpdata)
{
int len = 0;
int offset = 0;
int apdu_len = 0;
if (!apdu)
return -1;
if (!apdu) {
return BACNET_STATUS_ERROR;
}
if (apdu_size <= 3) {
return BACNET_STATUS_ERROR;
}
/* optional checking - most likely was already done prior to this call */
if (apdu[0] != PDU_TYPE_COMPLEX_ACK)
return -1;
if (apdu[0] != PDU_TYPE_COMPLEX_ACK) {
return BACNET_STATUS_ERROR;
}
*invoke_id = apdu[1];
if (apdu[2] != SERVICE_CONFIRMED_READ_PROPERTY)
return -1;
offset = 3;
if (apdu_len > offset) {
if (apdu[2] != SERVICE_CONFIRMED_READ_PROPERTY) {
return BACNET_STATUS_ERROR;
}
apdu_len = 3;
if (apdu_size > apdu_len) {
len = rp_ack_decode_service_request(
&apdu[offset], apdu_len - offset, rpdata);
&apdu[apdu_len], apdu_size - apdu_len, rpdata);
if (len > 0) {
apdu_len += len;
} else {
apdu_len = len;
}
}
return len;
return apdu_len;
}
#if defined(CONFIG_ZTEST_NEW_API)
@@ -80,12 +102,11 @@ static void testReadPropertyAck(void)
{
uint8_t apdu[480] = { 0 };
uint8_t apdu2[480] = { 0 };
int len = 0;
int apdu_len = 0;
int apdu_len = 0, test_len = 0, null_len = 0;
uint8_t invoke_id = 1;
uint8_t test_invoke_id = 0;
BACNET_READ_PROPERTY_DATA rpdata;
BACNET_READ_PROPERTY_DATA test_data;
BACNET_READ_PROPERTY_DATA rpdata = { 0 };
BACNET_READ_PROPERTY_DATA test_data = { 0 };
BACNET_OBJECT_TYPE object_type = OBJECT_DEVICE;
uint32_t object_instance = 0;
BACNET_OBJECT_TYPE object = 0;
@@ -99,13 +120,16 @@ static void testReadPropertyAck(void)
&apdu2[0], rpdata.object_type, rpdata.object_instance);
rpdata.application_data = &apdu2[0];
len = rp_ack_encode_apdu(&apdu[0], invoke_id, &rpdata);
zassert_not_equal(len, 0, NULL);
zassert_not_equal(len, -1, NULL);
apdu_len = len;
len = rp_ack_decode_apdu(&apdu[0], apdu_len, /* total length of the apdu */
&test_invoke_id, &test_data);
zassert_not_equal(len, -1, NULL);
null_len = rp_ack_encode_apdu(NULL, invoke_id, &rpdata);
apdu_len = rp_ack_encode_apdu(&apdu[0], invoke_id, &rpdata);
zassert_equal(
apdu_len, null_len, "apdu_len=%d null_len=%d", apdu_len, null_len);
zassert_not_equal(apdu_len, 0, NULL);
zassert_not_equal(apdu_len, BACNET_STATUS_ERROR, NULL);
test_len =
rp_ack_decode_apdu(&apdu[0], apdu_len, &test_invoke_id, &test_data);
zassert_equal(apdu_len, test_len, NULL);
zassert_not_equal(test_len, -1, NULL);
zassert_equal(test_invoke_id, invoke_id, NULL);
zassert_equal(test_data.object_type, rpdata.object_type, NULL);
@@ -117,11 +141,22 @@ static void testReadPropertyAck(void)
/* since object property == object_id, decode the application data using
the appropriate decode function */
len =
test_len =
decode_object_id(test_data.application_data, &object, &object_instance);
object_type = object;
zassert_equal(object_type, rpdata.object_type, NULL);
zassert_equal(object_instance, rpdata.object_instance, NULL);
while (apdu_len) {
apdu_len--;
if ((apdu_len <= 15) && (apdu_len >= 11)) {
/* boundary of optional parameters, so becomes valid */
continue;
}
test_len =
rp_ack_decode_apdu(&apdu[0], apdu_len, &test_invoke_id, &test_data);
zassert_true(
test_len < 0, "test_len=%d apdu_len=%d", test_len, apdu_len);
}
}
#if defined(CONFIG_ZTEST_NEW_API)
@@ -131,8 +166,7 @@ static void testReadProperty(void)
#endif
{
uint8_t apdu[480] = { 0 };
int len = 0;
int apdu_len = 0;
int apdu_len = 0, test_len = 0, null_len = 0;
uint8_t invoke_id = 128;
uint8_t test_invoke_id = 0;
BACNET_READ_PROPERTY_DATA rpdata;
@@ -142,16 +176,27 @@ static void testReadProperty(void)
rpdata.object_instance = 1;
rpdata.object_property = PROP_OBJECT_IDENTIFIER;
rpdata.array_index = BACNET_ARRAY_ALL;
len = rp_encode_apdu(&apdu[0], invoke_id, &rpdata);
zassert_not_equal(len, 0, NULL);
apdu_len = len;
null_len = rp_encode_apdu(NULL, invoke_id, &rpdata);
apdu_len = rp_encode_apdu(&apdu[0], invoke_id, &rpdata);
zassert_equal(
apdu_len, null_len, "apdu_len=%d null_len=%d", apdu_len, null_len);
zassert_not_equal(apdu_len, 0, NULL);
len = rp_decode_apdu(&apdu[0], apdu_len, &test_invoke_id, &test_data);
zassert_not_equal(len, -1, NULL);
test_len = rp_decode_apdu(&apdu[0], apdu_len, &test_invoke_id, &test_data);
zassert_equal(
apdu_len, test_len, "apdu_len=%d test_len=%d", apdu_len, test_len);
zassert_not_equal(test_len, -1, NULL);
zassert_equal(test_data.object_type, rpdata.object_type, NULL);
zassert_equal(test_data.object_instance, rpdata.object_instance, NULL);
zassert_equal(test_data.object_property, rpdata.object_property, NULL);
zassert_equal(test_data.array_index, rpdata.array_index, NULL);
while (apdu_len) {
apdu_len--;
test_len =
rp_decode_apdu(&apdu[0], apdu_len, &test_invoke_id, &test_data);
zassert_true(
test_len < 0, "test_len=%d apdu_len=%d", test_len, apdu_len);
}
return;
}
@@ -159,16 +204,13 @@ static void testReadProperty(void)
* @}
*/
#if defined(CONFIG_ZTEST_NEW_API)
ZTEST_SUITE(rp_tests, NULL, NULL, NULL, NULL, NULL);
#else
void test_main(void)
{
ztest_test_suite(rp_tests,
ztest_unit_test(testReadProperty),
ztest_unit_test(testReadPropertyAck)
);
ztest_test_suite(rp_tests, ztest_unit_test(testReadProperty),
ztest_unit_test(testReadPropertyAck));
ztest_run_test_suite(rp_tests);
}