1
0
mirror of https://github.com/RIOT-OS/RIOT.git synced 2024-12-29 04:50:03 +01:00
18746: sys/clif: Fixing out of bounds read under certain conditions r=maribu a=Teufelchen1

Hi 😈

This fixes a potential out of bounds read in clif_encode_link. There is no code in RIOT that can be exploited.
The fix does not break the current API but alters the behaviour slightly. Before the change, the length attributes of `clif_attr_t` where optional. If missing, the length was deduced using `strlen()`. This fix makes those parameters required and if they are `0` it operates as if the length really is `0`. This might not be ideal but it is the only non api breaking fix I could think off. 
```c
typedef struct {
    char *value;                  
    unsigned value_len;    NO LONGER OPTIONAL
    const char *key;               
    unsigned key_len;       NO LONGER OPTIONAL
} clif_attr_t;
```
Depends on #18744

cc `@leandrolanzieri` 

19161: bors.yaml: re-activate labels check + add block_labels r=miri64 a=miri64



Co-authored-by: Teufelchen1 <bennet.blischke@haw-hamburg.de>
Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
This commit is contained in:
bors[bot] 2023-02-23 16:39:44 +00:00 committed by GitHub
commit 72a0f1972d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 16 additions and 24 deletions

View File

@ -3,8 +3,7 @@
pr_status = [
"python-tests",
"tools-build-success",
# temporarily disabling while testing updated check-labels-action (#19101)
# "check-labels",
"check-labels",
"static-tests",
"check-commits (commit-msg)",
"check-commits (pr_check)",
@ -19,6 +18,8 @@ status = [
"tools-build-success",
]
block_labels = [ "Process: missing approvals" ]
# Number of project members who must approve the PR (using GitHub Reviews)
# before it is pushed to master.
# This necessary even with the check-labels action (which checks for >1 ACKs),

View File

@ -177,22 +177,17 @@ ssize_t clif_add_attr(clif_attr_t *attr, char *buf, size_t maxlen)
assert(attr);
assert(attr->key);
/* if no length given, calculate it */
if (!attr->key_len) {
attr->key_len = strlen(attr->key);
}
/* count attr name size and separator ';' */
size_t req_space = attr->key_len + 1;
size_t pos = 0;
int quoted = strcmp(attr->key, LF_ATTR_SIZE) ? 1 : 0;
int quoted = 0;
if (attr->key_len >= LF_ATTR_SIZE_S) {
quoted = strcmp(attr->key, LF_ATTR_SIZE) ? 1 : 0;
}
if (attr->value) {
if (!attr->value_len) {
attr->value_len = strlen(attr->value);
}
/* count also '=' */
req_space += attr->value_len + 1;
req_space += attr->value_len + 1;
}
if (quoted) {

View File

@ -237,9 +237,7 @@ ssize_t clif_add_target(const char *target, char *buf, size_t maxlen);
* @note
* - If @p buf is NULL this will return the amount of bytes that would be
* needed.
* - If the lengths of the key or the value of the attribute are not
* defined a NULL-terminated string will be assumed, and it will be
* calculated.
* - The length of the key must be set in `attr->key_len`.
*
* @return amount of bytes used from the buffer if successful
* @return CLIF_NO_SPACE if there is not enough space in the buffer

View File

@ -82,9 +82,9 @@ static void test_clif_encode_links(void)
const char exp_string[] = "</sensor/temp>;rt=\"temperature\";if=\"sensor\","
"</node/info>,</node/ep>;ct=\"40\"";
clif_attr_t attrs[] = {
{ .key = "rt", .value = "temperature" },
{ .key = "if", .value = "sensor" },
{ .key = "ct", .value = "40" }
{ .key = "rt", .key_len = 2, .value = "temperature", .value_len = 11 },
{ .key = "if", .key_len = 2, .value = "sensor", .value_len = 6 },
{ .key = "ct", .key_len = 2, .value = "40", .value_len = 2 }
};
clif_t links[] = {
@ -317,12 +317,10 @@ static void test_clif_get_attr_empty(void)
static void tests_clif_decode_encode_minimal(void)
{
char input_buf[] = "</sensors>";
#define BUFF_SIZE 50
/* The required buffer size is (in this case) equal to the input buffer
* plus one additional byte to hold the null termination */
const size_t output_buf_size = strlen(input_buf) + 1;
char output_buf[output_buf_size];
char input_buf[] = "</sensors>";
char output_buf[BUFF_SIZE];
clif_t out_link;
ssize_t input_len = strlen(input_buf);
@ -332,7 +330,7 @@ static void tests_clif_decode_encode_minimal(void)
TEST_FAIL("Malformed input string");
}
ssize_t result_len = clif_encode_link(&out_link, output_buf, output_buf_size);
ssize_t result_len = clif_encode_link(&out_link, output_buf, BUFF_SIZE);
if (result_len == CLIF_NO_SPACE) {
TEST_FAIL("No space left in the buffer");
}