Skip to content

Commit

Permalink
security: fix coverity issues
Browse files Browse the repository at this point in the history 
319210: Unitialized variables in oc_oscore_engine.c

319251: Resource leak in oc_oscore_context.c

319203, 319207, 319216, 319222, 319231, 319232, 319246, 319258:
  Resource leak in oc_obt.c

319217, 319227, 319229: Out-of-bounds access in
  oc_conv_byte_array_to_hex_string

319228: Uninitialized scalar variable in oc_obt.c
  • Loading branch information
@Danielius1922
Danielius1922 committed Jun 27, 2023
1 parent 28a03e3 commit c6b5c94
Show file tree
Hide file tree
Showing 8 changed files with 445 additions and 388 deletions.
11 changes: 5 additions & 6 deletions api/oc_helpers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,14 +325,13 @@ oc_conv_byte_array_to_hex_string(const uint8_t *array, size_t array_len,
if (*hex_str_len < array_len * 2 + 1) {
return -1;
}

*hex_str_len = 0;
size_t hlen = 0;
for (size_t i = 0; i < array_len; i++) {
snprintf(hex_str + *hex_str_len, 3, "%02x", array[i]);
*hex_str_len += 2;
snprintf(hex_str + hlen, 3, "%02x", array[i]);
hlen += 2;
}
hex_str[*hex_str_len++] = '\0';

hex_str[hlen] = '\0';
*hex_str_len = hlen;
return 0;
}

Expand Down
29 changes: 18 additions & 11 deletions include/oc_helpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@
#ifndef OC_HELPERS_H
#define OC_HELPERS_H

#include "oc_export.h"
#include "util/oc_compiler.h"
#include "util/oc_list.h"
#include "util/oc_mmem.h"
#include "oc_export.h"

#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
Expand Down Expand Up @@ -282,7 +284,8 @@ void _oc_free_array(
* @param str_len size of the string
*/
OC_API
void oc_set_string(oc_string_t *dst, const char *str, size_t str_len);
void oc_set_string(oc_string_t *dst, const char *str, size_t str_len)
OC_NONNULL(1);

/**
* @brief copy ocstring
Expand All @@ -292,7 +295,7 @@ void oc_set_string(oc_string_t *dst, const char *str, size_t str_len);
* is memset to zeroes)
*/
OC_API
void oc_copy_string(oc_string_t *dst, const oc_string_t *src);
void oc_copy_string(oc_string_t *dst, const oc_string_t *src) OC_NONNULL(1);

/**
* @brief new array
Expand Down Expand Up @@ -324,28 +327,32 @@ void _oc_alloc_string_array(
/**
* @brief convert array to hex
*
* @param[in] array array of bytes
* @param[in] array array of bytes (cannot be NULL)
* @param[in] array_len length of the array
* @param hex_str data as hex
* @param hex_str_len lenght of the hex string
* @param[out] hex_str data as hex (cannot be NULL)
* @param[in,out] hex_str_len in: size of the hex_str array, out: string length
* of the output hex string (cannot be NULL)
* @return int 0 success
* @return int -1 on failure
*/
int oc_conv_byte_array_to_hex_string(const uint8_t *array, size_t array_len,
char *hex_str, size_t *hex_str_len);
char *hex_str, size_t *hex_str_len)
OC_NONNULL();

/**
* @brief convert hex string to byte array
*
* @param[in] hex_str hex string input
* @param[in] hex_str hex string input (cannot be NULL)
* @param[in] hex_str_len size of the hex string
* @param array array of bytes
* @param array_len byte array
* @param[out] array array of bytes (cannot be NULL)
* @param[in,out] array_len in: size of the of the \p array, out: length of the
* output array (cannot be NULL)
* @return int 0 success
* @return int -1 on failure
*/
int oc_conv_hex_string_to_byte_array(const char *hex_str, size_t hex_str_len,
uint8_t *array, size_t *array_len);
uint8_t *array, size_t *array_len)
OC_NONNULL();

#ifdef __cplusplus
}
Expand Down
2 changes: 1 addition & 1 deletion onboarding_tool/obtmain.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1637,7 +1637,6 @@ provision_ace2(void)
i = 0;
while (i < num_resources) {
oc_ace_res_t *res = oc_obt_ace_new_resource(ace);

if (!res) {
OC_PRINTF("\nERROR: Could not allocate new resource for ACE\n");
oc_obt_free_ace(ace);
Expand Down Expand Up @@ -1715,6 +1714,7 @@ provision_ace2(void)
OC_PRINTF("\nSuccessfully issued request to provision ACE\n");
} else {
OC_PRINTF("\nERROR issuing request to provision ACE\n");
oc_obt_free_ace(ace);
}
}

Expand Down
4 changes: 4 additions & 0 deletions python/oc_python.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1312,6 +1312,7 @@ py_provision_ace_cloud_access(const char *uuid)
OC_PRINTF("[C] Successfully issued request to provision ACE\n");
} else {
OC_PRINTF("[C] ERROR issuing request to provision ACE\n");
oc_obt_free_ace(ace);
}
}

Expand Down Expand Up @@ -1347,6 +1348,7 @@ py_provision_ace_to_obt(const char *uuid, const char *res_uri)
OC_PRINTF("[C] Successfully issued request to provision ACE %s\n", res_uri);
} else {
OC_PRINTF("[C] ERROR issuing request to provision ACE %s\n", res_uri);
oc_obt_free_ace(ace);
}
}

Expand Down Expand Up @@ -1385,6 +1387,7 @@ py_provision_ace_device_resources(const char *device_uuid,
OC_PRINTF("[C] Successfully issued request to provision ACE\n");
} else {
OC_PRINTF("[C] ERROR issuing request to provision ACE\n");
oc_obt_free_ace(ace);
}
}

Expand Down Expand Up @@ -1463,6 +1466,7 @@ py_provision_ace2(const char *target, const char *subject, const char *href,
OC_PRINTF("[C] Successfully issued request to provision ACE\n");
} else {
OC_PRINTF("[C] ERROR issuing request to provision ACE\n");
oc_obt_free_ace(ace);
}
}

Expand Down
80 changes: 49 additions & 31 deletions security/oc_cred.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,8 @@ oc_cred_read_credusage(oc_sec_credusage_t credusage)
const char *
oc_cred_read_encoding(oc_sec_encoding_t encoding)
{
// TODO: use oc_string_view_t

switch (encoding) {
case OC_ENCODING_BASE64:
return OC_ENCODING_BASE64_STR;
Expand Down Expand Up @@ -897,24 +899,29 @@ oc_sec_encode_roles(const oc_tls_peer_t *client, size_t device,
oc_rep_set_int(roles, credtype, cr->credtype);
/* credusage */
const char *credusage_string = oc_cred_read_credusage(cr->credusage);
if (strlen(credusage_string) > 4) {
oc_rep_set_text_string(roles, credusage, credusage_string);
size_t credusage_string_len = strlen(credusage_string);
if (credusage_string_len > 4) {
oc_rep_set_text_string_v1(roles, credusage, credusage_string,
credusage_string_len);
}
/* publicdata */
if (oc_string_len(cr->publicdata.data) > 0) {
oc_rep_set_object(roles, publicdata);
if (cr->publicdata.encoding == OC_ENCODING_PEM) {
oc_rep_set_text_string(publicdata, data,
oc_string(cr->publicdata.data));
oc_rep_set_text_string_v1(publicdata, data,
oc_string(cr->publicdata.data),
oc_string_len(cr->publicdata.data));
} else {
oc_rep_set_byte_string(publicdata, data,
oc_cast(cr->publicdata.data, const uint8_t),
oc_string_len(cr->publicdata.data));
}
const char *encoding_string =
oc_cred_read_encoding(cr->publicdata.encoding);
if (strlen(encoding_string) > 7) {
oc_rep_set_text_string(publicdata, encoding, encoding_string);
size_t encoding_string_len = strlen(encoding_string);
if (encoding_string_len > 7) {
oc_rep_set_text_string_v1(publicdata, encoding, encoding_string,
encoding_string_len);
}
oc_rep_close_object(roles, publicdata);
}
Expand Down Expand Up @@ -946,7 +953,7 @@ oc_sec_encode_cred(size_t device, oc_interface_mask_t iface_mask,
oc_rep_set_int(creds, credtype, cr->credtype);
/* subjectuuid */
if (cr->subjectuuid.id[0] == '*') {
oc_rep_set_text_string(creds, subjectuuid, "*");
oc_rep_set_text_string_v1(creds, subjectuuid, "*", 1);
} else {
oc_uuid_to_str(&cr->subjectuuid, uuid, OC_UUID_LEN);
oc_rep_set_text_string(creds, subjectuuid, uuid);
Expand All @@ -955,10 +962,12 @@ oc_sec_encode_cred(size_t device, oc_interface_mask_t iface_mask,
if ((to_storage || cr->credtype == OC_CREDTYPE_PSK) &&
oc_string_len(cr->role.role) > 0) {
oc_rep_set_object(creds, roleid);
oc_rep_set_text_string(roleid, role, oc_string(cr->role.role));
oc_rep_set_text_string_v1(roleid, role, oc_string(cr->role.role),
oc_string_len(cr->role.role));
if (oc_string_len(cr->role.authority) > 0) {
oc_rep_set_text_string(roleid, authority,
oc_string(cr->role.authority));
oc_rep_set_text_string_v1(roleid, authority,
oc_string(cr->role.authority),
oc_string_len(cr->role.authority));
}
oc_rep_close_object(creds, roleid);
}
Expand All @@ -970,46 +979,49 @@ oc_sec_encode_cred(size_t device, oc_interface_mask_t iface_mask,
oc_cast(cr->privatedata.data, const uint8_t),
oc_string_len(cr->privatedata.data));
} else {
oc_rep_set_text_string(privatedata, data,
oc_string(cr->privatedata.data));
oc_rep_set_text_string_v1(privatedata, data,
oc_string(cr->privatedata.data),
oc_string_len(cr->privatedata.data));
}
} else {
if (cr->privatedata.encoding == OC_ENCODING_RAW) {
oc_rep_set_byte_string(privatedata, data,
oc_cast(cr->privatedata.data, const uint8_t), 0);
} else {
oc_rep_set_text_string(privatedata, data, "");
oc_rep_set_text_string_v1(privatedata, data, "", 0);
}
}
const char *encoding_string =
oc_cred_read_encoding(cr->privatedata.encoding);
if (strlen(encoding_string) > 7) {
oc_rep_set_text_string(privatedata, encoding, encoding_string);
} else {
oc_rep_set_text_string(privatedata, encoding, OC_ENCODING_RAW_STR);
size_t encoding_string_len = strlen(encoding_string);
if (encoding_string_len <= 7) {
encoding_string = OC_ENCODING_RAW_STR;
encoding_string_len = OC_CHAR_ARRAY_LEN(OC_ENCODING_RAW_STR);
}
oc_rep_set_text_string_v1(privatedata, encoding, encoding_string,
encoding_string_len);
oc_rep_close_object(creds, privatedata);
#ifdef OC_OSCORE
/* oscore */
if (cr->oscore_ctx) {
oc_oscore_context_t *oscore_ctx = (oc_oscore_context_t *)cr->oscore_ctx;
char hex_str[OSCORE_CTXID_LEN * 2 + 1];
size_t hex_str_len;
oc_rep_set_object(creds, oscore);
if (cr->credtype != OC_CREDTYPE_OSCORE_MCAST_SERVER) {
hex_str_len = OSCORE_CTXID_LEN * 2 + 1;
size_t hex_str_len = sizeof(hex_str);
oc_conv_byte_array_to_hex_string(
oscore_ctx->sendid, oscore_ctx->sendid_len, hex_str, &hex_str_len);
oc_rep_set_text_string(oscore, senderid, hex_str);
oc_rep_set_text_string_v1(oscore, senderid, hex_str, hex_str_len);
}
if (cr->credtype != OC_CREDTYPE_OSCORE_MCAST_CLIENT) {
hex_str_len = OSCORE_CTXID_LEN * 2 + 1;
size_t hex_str_len = sizeof(hex_str);
oc_conv_byte_array_to_hex_string(
oscore_ctx->recvid, oscore_ctx->recvid_len, hex_str, &hex_str_len);
oc_rep_set_text_string(oscore, recipientid, hex_str);
oc_rep_set_text_string_v1(oscore, recipientid, hex_str, hex_str_len);
}
if (cr->credtype != OC_CREDTYPE_OSCORE) {
oc_rep_set_text_string(oscore, desc, oc_string(oscore_ctx->desc));
oc_rep_set_text_string_v1(oscore, desc, oc_string(oscore_ctx->desc),
oc_string_len(oscore_ctx->desc));
}
if (cr->credtype != OC_CREDTYPE_OSCORE_MCAST_SERVER) {
oc_rep_set_int(oscore, ssn, oscore_ctx->ssn);
Expand All @@ -1020,31 +1032,37 @@ oc_sec_encode_cred(size_t device, oc_interface_mask_t iface_mask,
#ifdef OC_PKI
/* credusage */
const char *credusage_string = oc_cred_read_credusage(cr->credusage);
if (strlen(credusage_string) > 4) {
oc_rep_set_text_string(creds, credusage, credusage_string);
size_t credusage_string_len = strlen(credusage_string);
if (credusage_string_len > 4) {
oc_rep_set_text_string_v1(creds, credusage, credusage_string,
credusage_string_len);
}
/* publicdata */
if (oc_string_len(cr->publicdata.data) > 0) {
oc_rep_set_object(creds, publicdata);
if (cr->publicdata.encoding == OC_ENCODING_PEM) {
oc_rep_set_text_string(publicdata, data,
oc_string(cr->publicdata.data));
oc_rep_set_text_string_v1(publicdata, data,
oc_string(cr->publicdata.data),
oc_string_len(cr->publicdata.data));
} else {
oc_rep_set_byte_string(publicdata, data,
oc_cast(cr->publicdata.data, const uint8_t),
oc_string_len(cr->publicdata.data));
}
const char *encoding_string =
oc_cred_read_encoding(cr->publicdata.encoding);
if (strlen(encoding_string) > 7) {
oc_rep_set_text_string(publicdata, encoding, encoding_string);
size_t encoding_string_len = strlen(encoding_string);
if (encoding_string_len > 7) {
oc_rep_set_text_string_v1(publicdata, encoding, encoding_string,
encoding_string_len);
}
oc_rep_close_object(creds, publicdata);
}
if (to_storage) {
oc_rep_set_boolean(creds, owner_cred, cr->owner_cred);
if ((oc_string(cr->tag) != NULL) && (oc_string_len(cr->tag) > 0)) {
oc_rep_set_text_string(creds, tag, oc_string(cr->tag));
if (oc_string_len(cr->tag) > 0) {
oc_rep_set_text_string_v1(creds, tag, oc_string(cr->tag),
oc_string_len(cr->tag));
}
}
#endif /* OC_PKI */
Expand Down
Loading

0 comments on commit c6b5c94

Please sign in to comment.