gtlv: check memory bounds 3/3: encoding to str

See Id8d997c9d5e655ff1842ec69eab6c073875c6330

Related: CID#275417
Related: SYS#5599
Change-Id: I63d52a4f5dba32d3a3887dd9c5e42e1695fb2aa3

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 @@ int osmo_gtlvs_encode(struct osmo_gtlv_put *gtlv, const void *decoded_struct, si
 		      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 @@ int osmo_gtlvs_encode(struct osmo_gtlv_put *gtlv, const void *decoded_struct, si
  * \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 @@ int osmo_gtlvs_encode_to_str_buf(char *buf, size_t buflen, const void *decoded_s
 
 			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 @@ int osmo_gtlvs_encode_to_str_buf(char *buf, size_t buflen, const void *decoded_s
  * \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 @@ static void write_c()
 		"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 @@ struct osmo_gtlv_coding msg_ie_coding[] = {
 
 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 @@ int myproto_msg_to_str_buf(char *buf, size_t buflen, const struct myproto_msg *m
 {
 	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 @@ int myproto_msg_to_str_buf(char *buf, size_t buflen, const struct myproto_msg *m
 {
 	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;