osmocom
/
libosmo-pfcp
mirror of https://gerrit.osmocom.org/libosmo-pfcp
11
0
Fork 0
Code Issues Releases Wiki Activity

gtlv: check memory bounds 2/3: decoding TLV 

See Id8d997c9d5e655ff1842ec69eab6c073875c6330

Related: CID#275417
Related: SYS#5599
Change-Id: I841da89112ccf70fcd0f60eb902445fb1712eb48
This commit is contained in:
Neels Hofmeyr 2022-08-11 16:00:04 +02:00 committed by Neels Janosch Hofmeyr
parent cb724a3484
commit 92860a29cd
4 changed files with 50 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
include/osmocom/gtlv/gtlv_dec_enc.h
View File

@ -182,9 +182,10 @@ struct osmo_gtlv_coding {
*/ */
typedef void (*osmo_gtlv_err_cb)(void *data, void *decoded_struct, const char *file, int line, const char *fmt, ...); typedef void (*osmo_gtlv_err_cb)(void *data, void *decoded_struct, const char *file, int line, const char *fmt, ...);
int osmo_gtlvs_decode(void *decoded_struct, unsigned int obj_ofs, struct osmo_gtlv_load *gtlv, bool tlv_ordered, int osmo_gtlvs_decode(void *decoded_struct, size_t decoded_struct_size,
const struct osmo_gtlv_coding *ie_coding, unsigned int obj_ofs, struct osmo_gtlv_load *gtlv, bool tlv_ordered,
osmo_gtlv_err_cb err_cb, void *err_cb_data, const struct value_string *iei_strs); 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(struct osmo_gtlv_put *gtlv, const void *decoded_struct, size_t decoded_struct_size, int osmo_gtlvs_encode(struct osmo_gtlv_put *gtlv, const void *decoded_struct, size_t decoded_struct_size,
unsigned int obj_ofs, const struct osmo_gtlv_coding *ie_coding, osmo_gtlv_err_cb err_cb, unsigned int obj_ofs, const struct osmo_gtlv_coding *ie_coding, osmo_gtlv_err_cb err_cb,

67
src/libosmo-gtlv/gtlv_dec_enc.c
View File

@ -63,6 +63,11 @@ static const void *membof_const(const void *parent, size_t parent_size, unsigned
return memb; return memb;
} }
static void *membof(void *parent, size_t parent_size, unsigned int memb_ofs)
{
return (void *)membof_const((const void *)parent, parent_size, memb_ofs);
}
/*! Decode a TLV structure from raw data to a decoded struct, for unordered TLV IEs. /*! Decode a TLV structure from raw data to a decoded struct, for unordered TLV IEs.
* How to decode IE values and where to place them in the decoded struct, is defined by ie_coding, an array terminated * How to decode IE values and where to place them in the decoded struct, is defined by ie_coding, an array terminated
* by a '{}' entry. * by a '{}' entry.
@ -78,11 +83,13 @@ static const void *membof_const(const void *parent, size_t parent_size, unsigned
* \param[in] iei_strs value_string array to give IEI names in error messages passed to err_cb(), or NULL. * \param[in] iei_strs value_string array to give IEI names in error messages passed to err_cb(), or NULL.
* \return 0 on success, negative on error. * \return 0 on success, negative on error.
*/ */
static int osmo_gtlvs_decode_unordered(void *decoded_struct, unsigned int obj_ofs, struct osmo_gtlv_load *gtlv, static int osmo_gtlvs_decode_unordered(void *decoded_struct, size_t decoded_struct_size,
const struct osmo_gtlv_coding *ie_coding, unsigned int obj_ofs, struct osmo_gtlv_load *gtlv,
osmo_gtlv_err_cb err_cb, void *err_cb_data, const struct value_string *iei_strs) const struct osmo_gtlv_coding *ie_coding,
osmo_gtlv_err_cb err_cb, void *err_cb_data, const struct value_string *iei_strs)
{ {
void *obj = MEMB(decoded_struct, obj_ofs); void *obj = membof(decoded_struct, decoded_struct_size, obj_ofs);
size_t obj_maxlen = decoded_struct_size - obj_ofs;
const struct osmo_gtlv_coding *iec; const struct osmo_gtlv_coding *iec;
unsigned int *multi_count_p = NULL; unsigned int *multi_count_p = NULL;
@ -136,8 +143,10 @@ static int osmo_gtlvs_decode_unordered(void *decoded_struct, unsigned int obj_of
ie_max_allowed_count += iec->has_count ? iec->count_max : 1; ie_max_allowed_count += iec->has_count ? iec->count_max : 1;
/* Was this iec instance already decoded? Then skip to the next one, if any. */ /* Was this iec instance already decoded? Then skip to the next one, if any. */
presence_flag_p = iec->has_presence_flag ? MEMB(obj, iec->presence_flag_ofs) : NULL; presence_flag_p = iec->has_presence_flag ?
multi_count_p = iec->has_count ? MEMB(obj, iec->count_ofs) : NULL; membof(obj, obj_maxlen, iec->presence_flag_ofs) : NULL;
multi_count_p = iec->has_count ?
membof(obj, obj_maxlen, iec->count_ofs) : NULL;
if ((presence_flag_p && *presence_flag_p) if ((presence_flag_p && *presence_flag_p)
|| (multi_count_p && *multi_count_p >= iec->count_max)) || (multi_count_p && *multi_count_p >= iec->count_max))
continue; continue;
@ -187,15 +196,16 @@ static int osmo_gtlvs_decode_unordered(void *decoded_struct, unsigned int obj_of
default: default:
OSMO_ASSERT(0); OSMO_ASSERT(0);
} }
rc = osmo_gtlvs_decode(decoded_struct, obj_ofs + memb_ofs, &inner_tlv, ordered, iec->nested_ies, rc = osmo_gtlvs_decode(decoded_struct, decoded_struct_size,
err_cb, err_cb_data, iei_strs); obj_ofs + memb_ofs, &inner_tlv, ordered, iec->nested_ies,
err_cb, err_cb_data, iei_strs);
if (rc) if (rc)
RETURN_ERROR(rc, gtlv->ti, "Error while decoding TLV structure nested inside this IE"); RETURN_ERROR(rc, gtlv->ti, "Error while decoding TLV structure nested inside this IE");
} else { } else {
/* Normal IE, decode the specific IE data. */ /* Normal IE, decode the specific IE data. */
if (!iec->dec_func) if (!iec->dec_func)
RETURN_ERROR(-EIO, gtlv->ti, "IE definition lacks a dec_func()"); RETURN_ERROR(-EIO, gtlv->ti, "IE definition lacks a dec_func()");
rc = iec->dec_func(decoded_struct, MEMB(obj, memb_ofs), gtlv); rc = iec->dec_func(decoded_struct, membof(obj, obj_maxlen, memb_ofs), gtlv);
if (rc) if (rc)
RETURN_ERROR(rc, gtlv->ti, "Error while decoding this IE"); RETURN_ERROR(rc, gtlv->ti, "Error while decoding this IE");
} }
@ -218,7 +228,7 @@ static int osmo_gtlvs_decode_unordered(void *decoded_struct, unsigned int obj_of
for (iec = ie_coding; !osmo_gtlv_coding_end(iec); iec++) { for (iec = ie_coding; !osmo_gtlv_coding_end(iec); iec++) {
if (iec->has_presence_flag) if (iec->has_presence_flag)
continue; continue;
multi_count_p = iec->has_count ? MEMB(obj, iec->count_ofs) : NULL; multi_count_p = iec->has_count ? membof(obj, obj_maxlen, iec->count_ofs) : NULL;
if (multi_count_p) { if (multi_count_p) {
if (*multi_count_p < iec->count_mandatory) if (*multi_count_p < iec->count_mandatory)
RETURN_ERROR(-EINVAL, iec->ti, "%u instances of this IE are mandatory, got %u", RETURN_ERROR(-EINVAL, iec->ti, "%u instances of this IE are mandatory, got %u",
@ -247,18 +257,22 @@ static int osmo_gtlvs_decode_unordered(void *decoded_struct, unsigned int obj_of
* \param[in] iei_strs value_string array to give IEI names in error messages passed to err_cb(), or NULL. * \param[in] iei_strs value_string array to give IEI names in error messages passed to err_cb(), or NULL.
* \return 0 on success, negative on error. * \return 0 on success, negative on error.
*/ */
static int osmo_gtlvs_decode_ordered(void *decoded_struct, unsigned int obj_ofs, struct osmo_gtlv_load *gtlv, static int osmo_gtlvs_decode_ordered(void *decoded_struct, size_t decoded_struct_size,
const struct osmo_gtlv_coding *ie_coding, unsigned int obj_ofs, struct osmo_gtlv_load *gtlv,
osmo_gtlv_err_cb err_cb, void *err_cb_data, const struct value_string *iei_strs) const struct osmo_gtlv_coding *ie_coding,
osmo_gtlv_err_cb err_cb, void *err_cb_data, const struct value_string *iei_strs)
{ {
void *obj = MEMB(decoded_struct, obj_ofs); void *obj = membof(decoded_struct, decoded_struct_size, obj_ofs);
size_t obj_maxlen = decoded_struct_size - obj_ofs;
osmo_gtlv_load_start(gtlv); osmo_gtlv_load_start(gtlv);
for (; !osmo_gtlv_coding_end(ie_coding); ie_coding++) { for (; !osmo_gtlv_coding_end(ie_coding); ie_coding++) {
int rc; int rc;
bool *presence_flag = ie_coding->has_presence_flag ? MEMB(obj, ie_coding->presence_flag_ofs) : NULL; bool *presence_flag = ie_coding->has_presence_flag ?
unsigned int *multi_count = ie_coding->has_count ? MEMB(obj, ie_coding->count_ofs) : NULL; membof(obj, obj_maxlen, ie_coding->presence_flag_ofs) : NULL;
unsigned int *multi_count = ie_coding->has_count ?
membof(obj, obj_maxlen, ie_coding->count_ofs) : NULL;
struct osmo_gtlv_tag_inst peek_ti; struct osmo_gtlv_tag_inst peek_ti;
rc = osmo_gtlv_load_next_by_tag_inst(gtlv, &ie_coding->ti); rc = osmo_gtlv_load_next_by_tag_inst(gtlv, &ie_coding->ti);
@ -308,8 +322,9 @@ static int osmo_gtlvs_decode_ordered(void *decoded_struct, unsigned int obj_ofs,
default: default:
OSMO_ASSERT(0); OSMO_ASSERT(0);
} }
rc = osmo_gtlvs_decode(decoded_struct, obj_ofs + memb_ofs, &inner_tlv, ordered, rc = osmo_gtlvs_decode(decoded_struct, decoded_struct_size,
ie_coding->nested_ies, err_cb, err_cb_data, iei_strs); obj_ofs + memb_ofs, &inner_tlv, ordered, ie_coding->nested_ies,
err_cb, err_cb_data, iei_strs);
if (rc) if (rc)
RETURN_ERROR(rc, ie_coding->ti, RETURN_ERROR(rc, ie_coding->ti,
"Error while decoding TLV structure nested inside this IE"); "Error while decoding TLV structure nested inside this IE");
@ -317,7 +332,7 @@ static int osmo_gtlvs_decode_ordered(void *decoded_struct, unsigned int obj_ofs,
/* Normal IE, decode the specific IE data. */ /* Normal IE, decode the specific IE data. */
if (!ie_coding->dec_func) if (!ie_coding->dec_func)
RETURN_ERROR(-EIO, ie_coding->ti, "IE definition lacks a dec_func()"); RETURN_ERROR(-EIO, ie_coding->ti, "IE definition lacks a dec_func()");
rc = ie_coding->dec_func(decoded_struct, MEMB(obj, memb_ofs), gtlv); rc = ie_coding->dec_func(decoded_struct, membof(obj, obj_maxlen, memb_ofs), gtlv);
if (rc) if (rc)
RETURN_ERROR(rc, ie_coding->ti, "Error while decoding this IE"); RETURN_ERROR(rc, ie_coding->ti, "Error while decoding this IE");
} }
@ -365,17 +380,19 @@ static int osmo_gtlvs_decode_ordered(void *decoded_struct, unsigned int obj_ofs,
* \param[in] iei_strs value_string array to give IEI names in error messages passed to err_cb(), or NULL. * \param[in] iei_strs value_string array to give IEI names in error messages passed to err_cb(), or NULL.
* \return 0 on success, negative on error. * \return 0 on success, negative on error.
*/ */
int osmo_gtlvs_decode(void *decoded_struct, unsigned int obj_ofs, struct osmo_gtlv_load *gtlv, bool tlv_ordered, int osmo_gtlvs_decode(void *decoded_struct, size_t decoded_struct_size,
const struct osmo_gtlv_coding *ie_coding, unsigned int obj_ofs, struct osmo_gtlv_load *gtlv, bool tlv_ordered,
osmo_gtlv_err_cb err_cb, void *err_cb_data, const struct value_string *iei_strs) const struct osmo_gtlv_coding *ie_coding,
osmo_gtlv_err_cb err_cb, void *err_cb_data, const struct value_string *iei_strs)
{ {
if (!ie_coding) if (!ie_coding)
return -ENOTSUP; return -ENOTSUP;
if (tlv_ordered) if (tlv_ordered)
return osmo_gtlvs_decode_ordered(decoded_struct, obj_ofs, gtlv, ie_coding, err_cb, err_cb_data, iei_strs); return osmo_gtlvs_decode_ordered(decoded_struct, decoded_struct_size, obj_ofs, gtlv, ie_coding,
err_cb, err_cb_data, iei_strs);
else else
return osmo_gtlvs_decode_unordered(decoded_struct, obj_ofs, gtlv, ie_coding, err_cb, err_cb_data, return osmo_gtlvs_decode_unordered(decoded_struct, decoded_struct_size, obj_ofs, gtlv, ie_coding,
iei_strs); err_cb, err_cb_data, iei_strs);
} }
/*! Encode a TLV structure from decoded struct to raw data. /*! Encode a TLV structure from decoded struct to raw data.

3
src/libosmo-gtlv/gtlv_gen.c
View File

@ -381,7 +381,8 @@ static void write_c()
" %s message_type,\n" " %s message_type,\n"
" osmo_gtlv_err_cb err_cb, void *err_cb_data, const struct value_string *iei_strs)\n" " osmo_gtlv_err_cb err_cb, void *err_cb_data, const struct value_string *iei_strs)\n"
"{\n" "{\n"
" return osmo_gtlvs_decode(dst, 0, gtlv, tlv_ordered, %s_get_msg_coding(message_type), err_cb, err_cb_data, iei_strs);\n" " return osmo_gtlvs_decode(dst, sizeof(*dst), 0, gtlv, tlv_ordered, %s_get_msg_coding(message_type),\n"
" err_cb, err_cb_data, iei_strs);\n"
"}\n", "}\n",
g_cfg->proto_name, g_cfg->proto_name, g_cfg->message_type_enum ? : "int", g_cfg->proto_name); g_cfg->proto_name, g_cfg->proto_name, g_cfg->message_type_enum ? : "int", g_cfg->proto_name);
printf("\n" printf("\n"

3
tests/libosmo-gtlv/gtlv_dec_enc_test.c
View File

@ -396,7 +396,8 @@ void test_enc_dec(const char *label, const struct osmo_gtlv_cfg *cfg, bool order
.cfg = cfg, .cfg = cfg,
.src = { put.dst->data, put.dst->len }, .src = { put.dst->data, put.dst->len },
}; };
rc = osmo_gtlvs_decode(&parsed, 0, &load, ordered, msg_ie_coding, err_cb, &verify_err_cb_data, tag_names); rc = osmo_gtlvs_decode(&parsed, sizeof(parsed), 0, &load, ordered, msg_ie_coding,
err_cb, &verify_err_cb_data, tag_names);
printf("osmo_gtlvs_decode() rc = %d\n", rc); printf("osmo_gtlvs_decode() rc = %d\n", rc);
printf("decoded: %s\n", decoded_msg_to_str(&parsed)); printf("decoded: %s\n", decoded_msg_to_str(&parsed));
if (strcmp(decoded_msg_to_str(orig), decoded_msg_to_str(&parsed))) { if (strcmp(decoded_msg_to_str(orig), decoded_msg_to_str(&parsed))) {