neels has submitted this change. ( 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 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(-) Approvals: neels: Looks good to me, approved 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; null-- 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: 5 Gerrit-Owner: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged