Skip to content

Several bugfixes found by Claude #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
martinjaeger wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Several bugfixes found by Claude #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3a16e93
Binary mode: Only accept record items belonging to the record
martinjaeger Sep 22, 2026
ffbdc0c
Don't use the dummy check buffer for types described by a struct
martinjaeger Sep 22, 2026
25511ec
Text mode: Check buffer bounds when serializing values
martinjaeger Sep 23, 2026
59c188f
Release context lock when rejecting an unsupported export format
martinjaeger Sep 23, 2026
f6f88f3
Limit path parsing to the declared path length
martinjaeger Sep 23, 2026
f0f5769
Validate record indices before using them as an offset
martinjaeger Sep 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions src/thingset.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

LOG_MODULE_REGISTER(thingset, CONFIG_THINGSET_LOG_LEVEL);

Expand Down Expand Up @@ -194,7 +195,8 @@ int thingset_export_subsets(struct thingset_context *ts, uint8_t *buf, size_t bu
thingset_bin_setup(ts, 0);
break;
default:
return -THINGSET_ERR_NOT_IMPLEMENTED;
ret = -THINGSET_ERR_NOT_IMPLEMENTED;
goto out;
}

ret = ts->api->serialize_subsets(ts, subsets);
Expand All @@ -205,6 +207,7 @@ int thingset_export_subsets(struct thingset_context *ts, uint8_t *buf, size_t bu
ret = ts->rsp_pos;
}

out:
k_sem_give(&ts->lock);

return ret;
Expand Down Expand Up @@ -415,6 +418,14 @@ int thingset_import_record(struct thingset_context *ts, const uint8_t *data, siz

ts->endpoint = *endpoint;

if (ts->endpoint.object == NULL || ts->endpoint.object->type != THINGSET_TYPE_RECORDS
|| ts->endpoint.index < 0
|| ts->endpoint.index >= ts->endpoint.object->data.records->num_records)
{
err = -THINGSET_ERR_NOT_FOUND;
goto out;
}

switch (format) {
#ifdef CONFIG_THINGSET_TEXT_MODE
case THINGSET_TXT_NAMES_VALUES:
Expand All @@ -439,6 +450,12 @@ int thingset_import_record(struct thingset_context *ts, const uint8_t *data, siz
goto out;
}

/* dynamic records keep the data of only one record in memory */
struct thingset_records *records = ts->endpoint.object->data.records;
size_t record_offset = ts->endpoint.object->detail == THINGSET_DETAIL_DYN_RECORDS
? 0
: ts->endpoint.index * records->record_size;

const struct thingset_data_object *item;
while ((err = ts->api->deserialize_child(ts, &item)) != -THINGSET_ERR_DESERIALIZATION_FINISHED)
{
Expand All @@ -451,9 +468,7 @@ int thingset_import_record(struct thingset_context *ts, const uint8_t *data, siz
goto out;
}

struct thingset_records *records = ts->endpoint.object->data.records;
uint8_t *record_ptr =
(uint8_t *)records->records + ts->endpoint.index * records->record_size;
uint8_t *record_ptr = (uint8_t *)records->records + record_offset;
err = thingset_common_prepare_record_element(ts, item, record_ptr,
deserialize_value_callback);

Expand Down Expand Up @@ -621,10 +636,18 @@ struct thingset_data_object *thingset_get_object_by_path(struct thingset_context
const char *end;
uint16_t parent = 0;

if (path_len == 0) {
return NULL;
}

/* maximum depth of 10 assumed */
for (int i = 0; i < 10; i++) {
end = strchr(start, '/');
if (end == NULL || end >= path + path_len) {
/*
* The path may point directly into a received message and is not null-terminated, so the
* search for the separator has to be limited to the remaining length.
*/
end = memchr(start, '/', path + path_len - start);
if (end == NULL) {
/* reached at the end of the path */
if (object != NULL && object->type == THINGSET_TYPE_RECORDS && *start >= '0'
&& *start <= '9')
Expand All @@ -639,6 +662,10 @@ struct thingset_data_object *thingset_get_object_by_path(struct thingset_context
do {
if (*start >= '0' && *start <= '9') {
*index = (*index) * 10 + *start - '0';
if (*index > UINT16_MAX) {
/* record indices are limited to the range of uint16_t */
return NULL;
}
start++;
}
else {
Expand Down
20 changes: 11 additions & 9 deletions src/thingset_bin.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ static int bin_serialize_simple_value(zcbor_state_t *encoder, union thingset_dat
success = zcbor_int32_put(encoder, *data.i8);
break;
case THINGSET_TYPE_F32:
if (IS_ENABLED(CONFIG_THINGSET_ENCODE_ZERO_DECIMAL_FLOATS_AS_INTEGERS) && detail == 0)
{ /* round to 0 decimals: use int */
if (IS_ENABLED(CONFIG_THINGSET_ENCODE_ZERO_DECIMAL_FLOATS_AS_INTEGERS) && detail == 0) {
/* round to 0 decimals: use int */
success = zcbor_int32_put(encoder, lroundf(*data.f32));
}
else {
Expand Down Expand Up @@ -710,10 +710,11 @@ static int bin_deserialize_value(struct thingset_context *ts,
int index = 0;
do {
/* using uint8_t pointer for byte-wise pointer arithmetics */
union thingset_data_pointer data = { .u8 = array->elements.u8 + index * type_size };
union thingset_data_pointer data = { .u8 = array->elements.u8
+ index * type_size };

err = bin_deserialize_simple_value(ts, data, array->element_type, array->decimals,
check_only);
err = bin_deserialize_simple_value(ts, data, array->element_type,
array->decimals, check_only);
if (err != 0) {
break;
}
Expand Down Expand Up @@ -744,15 +745,16 @@ static int bin_deserialize_value(struct thingset_context *ts,

while (zcbor_uint32_decode(ts->decoder, &id) && id < UINT16_MAX) {
struct thingset_data_object *element = thingset_get_object_by_id(ts, id);
if (element == NULL) {
/* only items belonging to this record may be used */
if (element == NULL || element->parent_id != object->id) {
zcbor_any_skip(ts->decoder, NULL);
continue;
}
union thingset_data_pointer data = { .u8 = ((uint8_t *)records->records)
+ (i * records->record_size)
+ element->data.offset };
+ (i * records->record_size)
+ element->data.offset };
err = bin_deserialize_simple_value(ts, data, element->type, element->detail,
check_only);
check_only);
}

success = zcbor_map_end_decode(ts->decoder);
Expand Down
6 changes: 4 additions & 2 deletions src/thingset_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ int thingset_common_serialize_record(struct thingset_context *ts,
size_t record_offset;
int err;

if (record_index >= records->num_records) {
if (record_index < 0 || record_index >= records->num_records) {
return -THINGSET_ERR_NOT_FOUND;
}

Expand Down Expand Up @@ -328,10 +328,12 @@ int thingset_common_update(struct thingset_context *ts)
/*
* Test format of simple data types (up to 64-bit) by deserializing the value into a dummy
* object of the same type. For string and byte buffers only the size of the buffers is
* checked.
* checked. Types described by a struct don't fit into the dummy buffer, so the data
* pointer of the actual object is used for them.
*/
uint8_t dummy_data[8];
uint8_t *data = object->type == THINGSET_TYPE_BYTES || object->type == THINGSET_TYPE_ARRAY
|| object->type == THINGSET_TYPE_RECORDS
? object->data.u8
: dummy_data;
struct thingset_data_object dummy_object = {
Expand Down
Loading
Loading