laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/43231?usp=email )
Change subject: gsm/ipa: reject t_len == 0 in ID_GET/ID_RESP TLV parsers ......................................................................
gsm/ipa: reject t_len == 0 in ID_GET/ID_RESP TLV parsers
ipa_ccm_id_get_parse() and ipa_ccm_id_resp_parse() only checked t_len against the remaining buffer length, so a zero-length tag (t_len == 0) passed the check. The subsequent "t_len - 1" then underflowed: dec->lv[t_tag].len (a uint16_t) became 65535, and cur/len were adjusted in the wrong direction, letting the loop walk past the buffer on the next iteration.
Reject t_len < 1 up front, mirroring the equivalent guard already present in ipa_ccm_idtag_parse_off().
Change-Id: Ic8e791dc588aec2aa4825ed92c51f86502b370a3 Reported-By: Adam Bedard adam.bedard@gmail.com Related: OS#7050 --- M src/gsm/ipa.c 1 file changed, 12 insertions(+), 0 deletions(-)
Approvals: laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified
diff --git a/src/gsm/ipa.c b/src/gsm/ipa.c index d554faf..50fa31f 100644 --- a/src/gsm/ipa.c +++ b/src/gsm/ipa.c @@ -174,6 +174,12 @@ t_len = *cur++; t_tag = *cur++;
+ if (t_len < 1) { + LOGPC(DLMI, LOGL_DEBUG, "\n"); + LOGP(DLMI, LOGL_ERROR, "The tag length is too short: %d < 1\n", t_len); + return -EINVAL; + } + if (t_len > len + 1) { LOGPC(DLMI, LOGL_DEBUG, "\n"); LOGP(DLMI, LOGL_ERROR, "The tag does not fit: %d > %d\n", t_len, len + 1); @@ -216,6 +222,12 @@ cur += 2; t_tag = *cur++;
+ if (t_len < 1) { + LOGPC(DLMI, LOGL_DEBUG, "\n"); + LOGP(DLMI, LOGL_ERROR, "The tag length is too short: %d < 1\n", t_len); + return -EINVAL; + } + if (t_len > len + 1) { LOGPC(DLMI, LOGL_DEBUG, "\n"); LOGP(DLMI, LOGL_ERROR, "The tag does not fit: %d > %d\n", t_len, len + 1);