neels submitted this change.
gtlv: check memory bounds 3/3: encoding to str
See Id8d997c9d5e655ff1842ec69eab6c073875c6330
Related: CID#275417
Related: SYS#5599
Change-Id: I63d52a4f5dba32d3a3887dd9c5e42e1695fb2aa3
---
M include/osmocom/gtlv/gtlv_dec_enc.h
M src/libosmo-gtlv/gtlv_dec_enc.c
M src/libosmo-gtlv/gtlv_gen.c
M tests/libosmo-gtlv/gtlv_dec_enc_test.c
M tests/libosmo-gtlv/test_gtlv_gen/gtlv_gen_test.c
M tests/libosmo-gtlv/test_tliv/tliv_test.c
6 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/include/osmocom/gtlv/gtlv_dec_enc.h b/include/osmocom/gtlv/gtlv_dec_enc.h
index 2126beb..e671022 100644
--- a/include/osmocom/gtlv/gtlv_dec_enc.h
+++ b/include/osmocom/gtlv/gtlv_dec_enc.h
@@ -191,10 +191,12 @@
unsigned int obj_ofs, const struct osmo_gtlv_coding *ie_coding, osmo_gtlv_err_cb err_cb,
void *err_cb_data, const struct value_string *iei_strs);
-int osmo_gtlvs_encode_to_str_buf(char *buf, size_t buflen, const void *decoded_struct, unsigned int obj_ofs,
- const struct osmo_gtlv_coding *ie_coding, const struct value_string *iei_strs);
-char *osmo_gtlvs_encode_to_str_c(void *ctx, const void *decoded_struct, unsigned int obj_ofs,
- const struct osmo_gtlv_coding *ie_coding, const struct value_string *iei_strs);
+int osmo_gtlvs_encode_to_str_buf(char *buf, size_t buflen,
+ const void *decoded_struct, size_t decoded_struct_size, unsigned int obj_ofs,
+ const struct osmo_gtlv_coding *ie_coding, const struct value_string *iei_strs);
+char *osmo_gtlvs_encode_to_str_c(void *ctx,
+ const void *decoded_struct, size_t decoded_struct_size, unsigned int obj_ofs,
+ const struct osmo_gtlv_coding *ie_coding, const struct value_string *iei_strs);
static inline bool osmo_gtlv_coding_end(const struct osmo_gtlv_coding *iec)
{
diff --git a/src/libosmo-gtlv/gtlv_dec_enc.c b/src/libosmo-gtlv/gtlv_dec_enc.c
index e1e6d45..58c0405 100644
--- a/src/libosmo-gtlv/gtlv_dec_enc.c
+++ b/src/libosmo-gtlv/gtlv_dec_enc.c
@@ -28,9 +28,6 @@
#include <osmocom/gtlv/gtlv_dec_enc.h>
-/* Reverse offsetof(): return the address of the struct member for a given osmo_gtlv_msg and member ofs_foo value. */
-#define MEMB(M, MEMB_OFS) ((void *)((char *)(M) + (MEMB_OFS)))
-
#define RETURN_ERROR(RC, TAG_INST, FMT, ARGS...) \
do {\
if (err_cb) { \
@@ -486,19 +483,23 @@
* \param[in] iei_strs value_string array to give IEI names in tag headers, or NULL.
* \return number of characters that would be written if the buffer is large enough, like snprintf().
*/
-int osmo_gtlvs_encode_to_str_buf(char *buf, size_t buflen, const void *decoded_struct, unsigned int obj_ofs,
- const struct osmo_gtlv_coding *ie_coding, const struct value_string *iei_strs)
+int osmo_gtlvs_encode_to_str_buf(char *buf, size_t buflen,
+ const void *decoded_struct, size_t decoded_struct_size, unsigned int obj_ofs,
+ const struct osmo_gtlv_coding *ie_coding, const struct value_string *iei_strs)
{
struct osmo_strbuf sb = { .buf = buf, .len = buflen };
- void *obj = MEMB(decoded_struct, obj_ofs);
+ const void *obj = membof_const(decoded_struct, decoded_struct_size, obj_ofs);
+ size_t obj_maxlen = decoded_struct_size - obj_ofs;
if (!ie_coding)
return -ENOTSUP;
for (; !osmo_gtlv_coding_end(ie_coding); ie_coding++) {
- bool *presence_flag_p = ie_coding->has_presence_flag ? MEMB(obj, ie_coding->presence_flag_ofs) : NULL;
- unsigned int *multi_count_p = ie_coding->has_count ? MEMB(obj, ie_coding->count_ofs) : NULL;
+ const bool *presence_flag_p = ie_coding->has_presence_flag ?
+ membof_const(obj, obj_maxlen, ie_coding->presence_flag_ofs) : NULL;
+ const unsigned int *multi_count_p = ie_coding->has_count ?
+ membof_const(obj, obj_maxlen, ie_coding->count_ofs) : NULL;
unsigned int n;
unsigned int i;
@@ -531,12 +532,14 @@
if (ie_coding->nested_ies) {
OSMO_STRBUF_PRINTF(sb, "{");
- OSMO_STRBUF_APPEND(sb, osmo_gtlvs_encode_to_str_buf, decoded_struct, obj_ofs + memb_ofs,
+ OSMO_STRBUF_APPEND(sb, osmo_gtlvs_encode_to_str_buf,
+ decoded_struct, decoded_struct_size, obj_ofs + memb_ofs,
ie_coding->nested_ies, iei_strs);
OSMO_STRBUF_PRINTF(sb, " }");
} else {
if (ie_coding->enc_to_str_func)
- OSMO_STRBUF_APPEND(sb, ie_coding->enc_to_str_func, MEMB(obj, memb_ofs));
+ OSMO_STRBUF_APPEND(sb, ie_coding->enc_to_str_func,
+ membof_const(obj, obj_maxlen, memb_ofs));
else
OSMO_STRBUF_PRINTF(sb, "(enc_to_str_func==NULL)");
}
@@ -557,8 +560,10 @@
* \param[in] iei_strs value_string array to give IEI names in tag headers, or NULL.
* \return human readable string.
*/
-char *osmo_gtlvs_encode_to_str_c(void *ctx, const void *decoded_struct, unsigned int obj_ofs,
- const struct osmo_gtlv_coding *ie_coding, const struct value_string *iei_strs)
+char *osmo_gtlvs_encode_to_str_c(void *ctx,
+ const void *decoded_struct, size_t decoded_struct_size, unsigned int obj_ofs,
+ const struct osmo_gtlv_coding *ie_coding, const struct value_string *iei_strs)
{
- OSMO_NAME_C_IMPL(ctx, 256, "ERROR", osmo_gtlvs_encode_to_str_buf, decoded_struct, obj_ofs, ie_coding, iei_strs)
+ OSMO_NAME_C_IMPL(ctx, 256, "ERROR", osmo_gtlvs_encode_to_str_buf, decoded_struct, decoded_struct_size,
+ obj_ofs, ie_coding, iei_strs)
}
diff --git a/src/libosmo-gtlv/gtlv_gen.c b/src/libosmo-gtlv/gtlv_gen.c
index 3b2b1f0..e374ea9 100644
--- a/src/libosmo-gtlv/gtlv_gen.c
+++ b/src/libosmo-gtlv/gtlv_gen.c
@@ -397,7 +397,7 @@
"int %s_ies_encode_to_str(char *buf, size_t buflen, const union %s_ies *src,\n"
" %s message_type, const struct value_string *iei_strs)\n"
"{\n"
- " return osmo_gtlvs_encode_to_str_buf(buf, buflen, src, 0, %s_get_msg_coding(message_type), iei_strs);\n"
+ " return osmo_gtlvs_encode_to_str_buf(buf, buflen, src, sizeof(*src), 0, %s_get_msg_coding(message_type), iei_strs);\n"
"}\n",
g_cfg->proto_name, g_cfg->proto_name, g_cfg->message_type_enum ? : "int", g_cfg->proto_name);
}
diff --git a/tests/libosmo-gtlv/gtlv_dec_enc_test.c b/tests/libosmo-gtlv/gtlv_dec_enc_test.c
index a542cfa..f0de7b0 100644
--- a/tests/libosmo-gtlv/gtlv_dec_enc_test.c
+++ b/tests/libosmo-gtlv/gtlv_dec_enc_test.c
@@ -286,7 +286,7 @@
char *decoded_msg_to_str(const struct decoded_msg *m)
{
- return osmo_gtlvs_encode_to_str_c(ctx, m, 0, msg_ie_coding, tag_names);
+ return osmo_gtlvs_encode_to_str_c(ctx, m, sizeof(*m), 0, msg_ie_coding, tag_names);
}
diff --git a/tests/libosmo-gtlv/test_gtlv_gen/gtlv_gen_test.c b/tests/libosmo-gtlv/test_gtlv_gen/gtlv_gen_test.c
index ef5372c..1997fdc 100644
--- a/tests/libosmo-gtlv/test_gtlv_gen/gtlv_gen_test.c
+++ b/tests/libosmo-gtlv/test_gtlv_gen/gtlv_gen_test.c
@@ -203,8 +203,8 @@
{
struct osmo_strbuf sb = { .buf = buf, .len = buflen };
OSMO_STRBUF_PRINTF(sb, "%s={", get_value_string(myproto_msg_type_names, m->type));
- OSMO_STRBUF_APPEND(sb, osmo_gtlvs_encode_to_str_buf, &m->ies, 0, myproto_get_msg_coding(m->type),
- myproto_iei_names);
+ OSMO_STRBUF_APPEND(sb, osmo_gtlvs_encode_to_str_buf, &m->ies, sizeof(m->ies), 0,
+ myproto_get_msg_coding(m->type), myproto_iei_names);
OSMO_STRBUF_PRINTF(sb, " }");
return sb.chars_needed;
diff --git a/tests/libosmo-gtlv/test_tliv/tliv_test.c b/tests/libosmo-gtlv/test_tliv/tliv_test.c
index fd4e310..49ae25c 100644
--- a/tests/libosmo-gtlv/test_tliv/tliv_test.c
+++ b/tests/libosmo-gtlv/test_tliv/tliv_test.c
@@ -101,8 +101,8 @@
{
struct osmo_strbuf sb = { .buf = buf, .len = buflen };
OSMO_STRBUF_PRINTF(sb, "%s={", get_value_string(myproto_msg_type_names, m->type));
- OSMO_STRBUF_APPEND(sb, osmo_gtlvs_encode_to_str_buf, &m->ies, 0, myproto_get_msg_coding(m->type),
- myproto_iei_names);
+ OSMO_STRBUF_APPEND(sb, osmo_gtlvs_encode_to_str_buf, &m->ies, sizeof(m->ies), 0,
+ myproto_get_msg_coding(m->type), myproto_iei_names);
OSMO_STRBUF_PRINTF(sb, " }");
return sb.chars_needed;
To view, visit change 29041. To unsubscribe, or for help writing mail filters, visit settings.