net: lwm2m: lwm2m_message_handling: fix validation of time. #85406
Conversation
| @@ -1160,7 +1160,7 @@ int lwm2m_write_handler(struct lwm2m_engine_obj_inst *obj_inst, struct lwm2m_eng | |||
| break; | |||
| } | |||
|
|
|||
| if (write_buf_len == sizeof(time_t)) { | |||
| if ((data_len == sizeof(time_t)) || (write_buf_len == sizeof(time_t))) { | |||
There was a problem hiding this comment.
Maybe, a cleaner solution would be to :
if (write_buf_len >= sizeof(time_t)) {With your modification, the destination buffer can be overrun. If the received data len (data_len) is 8 and the destination (write_buf_len) is 4 (uint32_t), it will write a data of 8 bytes inside a 4 bytes destination.
There was a problem hiding this comment.
I agree. Thanks
Currently the if statement checks if the write_buf_len is the sizeof(time_t), when CONFIG_LWM2M_ENGINE_VALIDATION_BUFFER_SIZE is > 0 it will always assume the the resource buf len is not supported unless CONFIG_LWM2M_ENGINE_VALIDATION_BUFFER_SIZE is equal to sizeof(time_t). To fix this the check should be CONFIG_LWM2M_ENGINE_VALIDATION_BUFFER_SIZE >= sizeof(time_t). Signed-off-by: Brandon Allen <brandon.allen@exacttechnology.com>
There was a problem hiding this comment.
I don't think the fix is correct - that way we ignore the actual data buffer size - i. e. the time value would always be interpreted as time_t even if the local storage only accepts 4 bytes.
From what I've checked the culprit is that we compare the validation buffer size with the size of the data storage type, we should rather compare against the actual data buffer size:
diff --git a/subsys/net/lib/lwm2m/lwm2m_message_handling.c b/subsys/net/lib/lwm2m/lwm2m_message_handling.c
index 9b67a8137a4..f56d7d5eef0 100644
--- a/subsys/net/lib/lwm2m/lwm2m_message_handling.c
+++ b/subsys/net/lib/lwm2m/lwm2m_message_handling.c
@@ -1163,14 +1163,14 @@ int lwm2m_write_handler(struct lwm2m_engine_obj_inst *obj_inst, struct lwm2m_eng
break;
}
- if (write_buf_len == sizeof(time_t)) {
+ if (data_len == sizeof(time_t)) {
*(time_t *)write_buf = temp_time;
len = sizeof(time_t);
- } else if (write_buf_len == sizeof(uint32_t)) {
+ } else if (data_len == sizeof(uint32_t)) {
*(uint32_t *)write_buf = (uint32_t)temp_time;
len = sizeof(uint32_t);
} else {
- LOG_ERR("Time resource buf len not supported %zu", write_buf_len);
+ LOG_ERR("Time resource buf len not supported %zu", data_len);
ret = -EINVAL;
}
fixes #85330