<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/13907">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">libmsc/gsm_04_08.c: refactor CM Service Request parsing<br><br>In gsm48_rx_mm_serv_req() we need to make sure that a given message<br>buffer is large enough to contain both 'gsm48_hdr' and<br>'gsm48_service_request' structures.<br><br>Comparing msg->data_len with size of pointer if wrong because:<br><br> - we actually need to compare with size of struct(s),<br> - we need msgb_l3len(), not length of the whole buffer.<br><br>Moreover, since we have to use the pointer arithmetics in order<br>to keep backwards compatibility with Phase1 phones, we also<br>need to check the length of both Classmark2 and MI IEs.<br><br>Change-Id: I6e7454d7a6f63fd5a0e12fb90d8c58688da0951e<br>---<br>M src/libmsc/gsm_04_08.c<br>1 file changed, 38 insertions(+), 19 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c</span><br><span>index 280c341..418ee2a 100644</span><br><span>--- a/src/libmsc/gsm_04_08.c</span><br><span>+++ b/src/libmsc/gsm_04_08.c</span><br><span>@@ -700,28 +700,47 @@</span><br><span> int gsm48_rx_mm_serv_req(struct msc_a *msc_a, struct msgb *msg)</span><br><span> {</span><br><span> struct gsm_network *net = msc_a_net(msc_a);</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t mi_type;</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm48_hdr *gh = msgb_l3(msg);</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm48_service_request *req =</span><br><span style="color: hsl(0, 100%, 40%);">- (struct gsm48_service_request *)gh->data;</span><br><span style="color: hsl(0, 100%, 40%);">- /* unfortunately in Phase1 the classmark2 length is variable */</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t classmark2_len = gh->data[1];</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t *classmark2_buf = gh->data+2;</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm48_classmark2 *cm2 = (void*)classmark2_buf;</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t *mi_p = classmark2_buf + classmark2_len;</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t mi_len = *mi_p;</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t *mi = mi_p + 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_hdr *gh;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_service_request *req;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_classmark2 *cm2;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t *cm2_buf, cm2_len;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t *mi_buf, mi_len;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t *mi, mi_type;</span><br><span> bool is_utran;</span><br><span> struct vlr_subscr *vsub;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (msg->data_len < sizeof(struct gsm48_service_request*)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Make sure that both header and CM Service Request fit into the buffer */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l3len(msg) < sizeof(*gh) + sizeof(*req)) {</span><br><span> LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: wrong message size (%u < %zu)\n",</span><br><span style="color: hsl(0, 100%, 40%);">- msg->data_len, sizeof(struct gsm48_service_request*));</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_l3len(msg), sizeof(*gh) + sizeof(*req));</span><br><span> return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_INCORRECT_MESSAGE);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (msg->data_len < req->mi_len + 6) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: message does not fit in packet\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ gh = (struct gsm48_hdr *) msgb_l3(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ req = (struct gsm48_service_request *) gh->data;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Unfortunately in Phase1 the Classmark2 length is variable, so we cannot</span><br><span style="color: hsl(120, 100%, 40%);">+ * just use gsm48_service_request struct, and need to parse it manually. */</span><br><span style="color: hsl(120, 100%, 40%);">+ cm2_len = gh->data[1];</span><br><span style="color: hsl(120, 100%, 40%);">+ cm2_buf = gh->data + 2;</span><br><span style="color: hsl(120, 100%, 40%);">+ cm2 = (struct gsm48_classmark2 *) cm2_buf;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Prevent buffer overrun: check the length of Classmark2 */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (cm2_buf + cm2_len > msg->tail) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: Classmark2 "</span><br><span style="color: hsl(120, 100%, 40%);">+ "length=%u is too big\n", cm2_len);</span><br><span style="color: hsl(120, 100%, 40%);">+ return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_INCORRECT_MESSAGE);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* MI (Mobile Identity) LV follows the Classmark2 */</span><br><span style="color: hsl(120, 100%, 40%);">+ mi_buf = cm2_buf + cm2_len;</span><br><span style="color: hsl(120, 100%, 40%);">+ mi_len = mi_buf[0];</span><br><span style="color: hsl(120, 100%, 40%);">+ mi = mi_buf + 1;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Prevent buffer overrun: check the length of MI */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (mi_buf + mi_len > msg->tail) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: Mobile Identity "</span><br><span style="color: hsl(120, 100%, 40%);">+ "length=%u is too big\n", cm2_len);</span><br><span> return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_INCORRECT_MESSAGE);</span><br><span> }</span><br><span> </span><br><span>@@ -759,9 +778,9 @@</span><br><span> return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_SRV_OPT_NOT_SUPPORTED);</span><br><span> </span><br><span> if (msc_a_is_accepted(msc_a))</span><br><span style="color: hsl(0, 100%, 40%);">- return cm_serv_reuse_conn(msc_a, mi_p, req->cm_service_type);</span><br><span style="color: hsl(120, 100%, 40%);">+ return cm_serv_reuse_conn(msc_a, mi_buf, req->cm_service_type);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, mi_p);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, mi_buf);</span><br><span> </span><br><span> msc_a_get(msc_a, msc_a_cm_service_type_to_use(req->cm_service_type));</span><br><span> </span><br><span>@@ -775,7 +794,7 @@</span><br><span> is_utran || net->authentication_required,</span><br><span> is_utran || net->a5_encryption_mask > 0x01,</span><br><span> req->cipher_key_seq,</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_gsm48_classmark2_is_r99(cm2, classmark2_len),</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_gsm48_classmark2_is_r99(cm2, cm2_len),</span><br><span> is_utran);</span><br><span> </span><br><span> /* From vlr_proc_acc_req() we expect an implicit dispatch of PR_ARQ_E_START we expect</span><br><span>@@ -787,7 +806,7 @@</span><br><span> }</span><br><span> </span><br><span> vsub->classmark.classmark2 = *cm2;</span><br><span style="color: hsl(0, 100%, 40%);">- vsub->classmark.classmark2_len = classmark2_len;</span><br><span style="color: hsl(120, 100%, 40%);">+ vsub->classmark.classmark2_len = cm2_len;</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13907">change 13907</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13907"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I6e7454d7a6f63fd5a0e12fb90d8c58688da0951e </div>
<div style="display:none"> Gerrit-Change-Number: 13907 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>