neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmo-pfcp/+/29041 )
Change subject: gtlv: check memory bounds 3/3: encoding to str
......................................................................
gtlv: check memory bounds 3/3: encoding to str
See Id8d997c9d5e655ff1842ec69eab6c073875c6330
Related: CID#275417
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(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-pfcp refs/changes/41/29041/1
diff --git a/include/osmocom/gtlv/gtlv_dec_enc.h
b/include/osmocom/gtlv/gtlv_dec_enc.h
index 243ad96..acb4ad4 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_max_len, 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_max_len, 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 e442baa..7e8d524 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) { \
@@ -482,19 +479,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_max_len, 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_max_len,
obj_ofs);
+ size_t obj_maxlen = decoded_struct_max_len - 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;
@@ -527,12 +528,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_max_len, 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)");
}
@@ -553,8 +556,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_max_len, 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_max_len,
+ 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 https://gerrit.osmocom.org/c/libosmo-pfcp/+/29041
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-pfcp
Gerrit-Branch: master
Gerrit-Change-Id: I63d52a4f5dba32d3a3887dd9c5e42e1695fb2aa3
Gerrit-Change-Number: 29041
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-MessageType: newchange
