Bugfix/deprecate decode tag number and value (#481)

* added or updated secure the BACnet primitive value decoders - the core codecs - named bacnet_x_decode(), bacnet_x_application_decode() and bacnet_x_context_decode where x is one of the 13 BACnet primitive value names.  The updated API includes an APDU size to prevent over-reading of an APDU buffer while decoding.  Improved or added unit test code coverage for the BACnet primitive value decoders.

* marked the insecure decoding API as 'deprecated' which is defined in src/bacnet/basic/sys/platform.h and can be disabled during a build. 

* added secure decoders for BACnetTimeValue, BACnetHostNPort, BACnetTimeStamp, BACnetAddress, and Weekly_Schedule and improved unit test code coverage.

* improved test code coverage for BACnet objects and properties.

* secured AtomicReadFile and AtomicWriteFile service decoders and improved unit test code coverage.

* secured BACnet Error service decoder and improved unit test code coverage.

---------

Co-authored-by: Steve Karg <skarg@users.sourceforge.net>
This commit is contained in:
Steve Karg
2023-09-08 11:39:27 -05:00
committed by GitHub
parent bc8c261153
commit f641aacddb
67 changed files with 6103 additions and 3145 deletions
+142 -83
View File
@@ -117,20 +117,20 @@ int bacapp_encode_timestamp(uint8_t *apdu, BACNET_TIMESTAMP *value)
{
int len = 0; /* length of each encoding */
if (value && apdu) {
if (value) {
switch (value->tag) {
case TIME_STAMP_TIME:
len = encode_context_time(&apdu[0], 0, &value->value.time);
len = encode_context_time(apdu, value->tag, &value->value.time);
break;
case TIME_STAMP_SEQUENCE:
len = encode_context_unsigned(
&apdu[0], 1, value->value.sequenceNum);
apdu, value->tag, value->value.sequenceNum);
break;
case TIME_STAMP_DATETIME:
len = bacapp_encode_context_datetime(
&apdu[0], 2, &value->value.dateTime);
apdu, value->tag, &value->value.dateTime);
break;
default:
@@ -160,109 +160,168 @@ int bacapp_encode_context_timestamp(
int len = 0; /* length of each encoding */
int apdu_len = 0;
if (value && apdu) {
len = encode_opening_tag(&apdu[apdu_len], tag_number);
if (value) {
len = encode_opening_tag(apdu, tag_number);
apdu_len += len;
len = bacapp_encode_timestamp(&apdu[apdu_len], value);
if (apdu) {
apdu += len;
}
len = bacapp_encode_timestamp(apdu, value);
apdu_len += len;
len = encode_closing_tag(&apdu[apdu_len], tag_number);
if (apdu) {
apdu += len;
}
len = encode_closing_tag(apdu, tag_number);
apdu_len += len;
}
return apdu_len;
}
/** Decode a time stamp from the given buffer.
*
/**
* @brief Decode a time stamp from the given buffer.
* @param apdu Pointer to the APDU buffer.
* @param apdu_size - the APDU buffer length
* @param value Pointer to the variable that shall
* take the time stamp values.
* @return number of bytes decoded, or BACNET_STATUS_ERROR if an error occurs
*/
int bacnet_timestamp_decode(
uint8_t *apdu, uint32_t apdu_size, BACNET_TIMESTAMP *value)
{
int len = 0;
int apdu_len = 0;
BACNET_TAG tag = { 0 };
BACNET_UNSIGNED_INTEGER unsigned_value = 0;
BACNET_TIME *btime = NULL;
BACNET_DATE_TIME *bdatetime = NULL;
if (!apdu) {
return BACNET_STATUS_ERROR;
}
len = bacnet_tag_decode(&apdu[apdu_len], apdu_size - apdu_len, &tag);
if (len <= 0) {
return BACNET_STATUS_ERROR;
}
if (value) {
value->tag = tag.number;
}
switch (tag.number) {
case TIME_STAMP_TIME:
if (value) {
btime = &value->value.time;
}
len = bacnet_time_context_decode(&apdu[apdu_len],
apdu_size - apdu_len, tag.number, btime);
if (len <= 0) {
return BACNET_STATUS_ERROR;
}
apdu_len += len;
break;
case TIME_STAMP_SEQUENCE:
len = bacnet_unsigned_context_decode(&apdu[apdu_len],
apdu_size - apdu_len, tag.number, &unsigned_value);
if (len <= 0) {
return BACNET_STATUS_ERROR;
}
apdu_len += len;
if (unsigned_value <= UINT16_MAX) {
if (value) {
value->value.sequenceNum = (uint16_t)unsigned_value;
}
} else {
return BACNET_STATUS_ERROR;
}
break;
case TIME_STAMP_DATETIME:
if (value) {
bdatetime = &value->value.dateTime;
}
len = bacnet_datetime_context_decode(&apdu[apdu_len],
apdu_size - apdu_len, tag.number, bdatetime);
if (len <= 0) {
return BACNET_STATUS_ERROR;
}
apdu_len += len;
break;
default:
return BACNET_STATUS_ERROR;
}
return apdu_len;
}
/**
* @brief Decode a time stamp from the given buffer.
* @param apdu Pointer to the APDU buffer.
* @param value Pointer to the variable that shall
* take the time stamp values.
*
* @return Bytes decoded or -1 on error.
* @return number of bytes decoded, or BACNET_STATUS_ERROR if an error occurs
* @deprecated use bacnet_timestamp_decode() instead
*/
int bacapp_decode_timestamp(uint8_t *apdu, BACNET_TIMESTAMP *value)
{
int len = 0;
int section_len;
uint32_t len_value_type;
BACNET_UNSIGNED_INTEGER unsigned_value;
if (apdu && value) {
section_len = decode_tag_number_and_value(
&apdu[len], &value->tag, &len_value_type);
if (-1 == section_len) {
return -1;
}
switch (value->tag) {
case TIME_STAMP_TIME:
if ((section_len = decode_context_bacnet_time(&apdu[len],
TIME_STAMP_TIME, &value->value.time)) == -1) {
return -1;
} else {
len += section_len;
}
break;
case TIME_STAMP_SEQUENCE:
if ((section_len = decode_context_unsigned(&apdu[len],
TIME_STAMP_SEQUENCE, &unsigned_value)) == -1) {
return -1;
} else {
if (unsigned_value <= 0xffff) {
len += section_len;
value->value.sequenceNum = (uint16_t)unsigned_value;
} else {
return -1;
}
}
break;
case TIME_STAMP_DATETIME:
if ((section_len = bacapp_decode_context_datetime(&apdu[len],
TIME_STAMP_DATETIME, &value->value.dateTime)) == -1) {
return -1;
} else {
len += section_len;
}
break;
default:
return -1;
}
}
return len;
return bacnet_timestamp_decode(apdu, MAX_APDU, value);
}
/** Decode a time stamp and check for
* opening and closing tags.
*
/**
* @brief Decode a time stamp and check for opening and closing tags.
* @param apdu Pointer to the APDU buffer.
* @param apdu_size - the APDU buffer length
* @param tag_number The tag number that shall
* hold the time stamp.
* @param value Pointer to the variable that shall
* take the time stamp values.
* @return number of bytes decoded, or BACNET_STATUS_ERROR if an error occurs
*/
int bacnet_timestamp_context_decode(uint8_t *apdu,
uint32_t apdu_size,
uint8_t tag_number,
BACNET_TIMESTAMP *value)
{
int len = 0;
int apdu_len = 0;
if (!bacnet_is_opening_tag_number(
&apdu[apdu_len], apdu_size - apdu_len, tag_number, &len)) {
return BACNET_STATUS_ERROR;
}
apdu_len += len;
len = bacnet_timestamp_decode(&apdu[apdu_len], apdu_size - apdu_len, value);
if (len < 0) {
return BACNET_STATUS_ERROR;
}
apdu_len += len;
if (!bacnet_is_closing_tag_number(
&apdu[apdu_len], apdu_size - apdu_len, tag_number, &len)) {
return BACNET_STATUS_ERROR;
}
apdu_len += len;
return apdu_len;
}
/**
* @brief Decode a time stamp and check for opening and closing tags.
* @param apdu Pointer to the APDU buffer.
* @param tag_number The tag number that shall
* hold the time stamp.
* @param value Pointer to the variable that shall
* take the time stamp values.
*
* @return Bytes decoded or -1 on error.
* @return number of bytes decoded, or BACNET_STATUS_ERROR if an error occurs
* @deprecated use bacnet_timestamp_context_decode() instead
*/
int bacapp_decode_context_timestamp(
uint8_t *apdu, uint8_t tag_number, BACNET_TIMESTAMP *value)
{
int len = 0;
int section_len;
const uint32_t apdu_size = MAX_APDU;
int len;
if (decode_is_opening_tag_number(&apdu[len], tag_number)) {
len++;
section_len = bacapp_decode_timestamp(&apdu[len], value);
if (section_len > 0) {
len += section_len;
if (decode_is_closing_tag_number(&apdu[len], tag_number)) {
len++;
} else {
return -1;
}
}
len = bacnet_timestamp_context_decode(apdu, apdu_size, tag_number, value);
if (len <= 0) {
len = BACNET_STATUS_ERROR;
}
return len;
}
@@ -334,9 +393,9 @@ bool bacapp_timestamp_init_ascii(BACNET_TIMESTAMP *timestamp, const char *ascii)
}
}
if (!status) {
count = sscanf(ascii, "%4d", &sequence);
count = sscanf(ascii, "%5d", &sequence);
if (count == 1) {
timestamp->tag = TIME_STAMP_DATETIME;
timestamp->tag = TIME_STAMP_SEQUENCE;
timestamp->value.sequenceNum = (uint16_t)sequence;
status = true;
}