<p>laforge has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/19131">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">subchan_demux: Fix out-of-bounds write<br><br>We cannot blindly append two ubits to the 320-ubit sized buffer. In the<br>end, we may already fill the buffer after the first ubit, causing a<br>buffer overflow with the second ubit.<br><br>Lets check if the buffer is full after every bit. Avoid copy+pasting<br>but move the code repeated per bit to a new function.<br><br>Change-Id: I58d946265372278051e4f29301d4f201ab98c0fc<br>Closes: OS#4648<br>---<br>M src/subchan_demux.c<br>1 file changed, 25 insertions(+), 23 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/31/19131/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/subchan_demux.c b/src/subchan_demux.c</span><br><span>index 55503db..a3a44d9 100644</span><br><span>--- a/src/subchan_demux.c</span><br><span>+++ b/src/subchan_demux.c</span><br><span>@@ -92,6 +92,29 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static void append_bit_resync_out(struct subch_demux *dmx, int c, ubit_t bit)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct demux_subch *sch = &dmx->subch[c];</span><br><span style="color: hsl(120, 100%, 40%);">+ append_bit(sch, bit);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sync_hdr_complete(sch, bit))</span><br><span style="color: hsl(120, 100%, 40%);">+ resync_to_here(sch);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* FIXME: verify the first bit in octet 2, 4, 6, ...</span><br><span style="color: hsl(120, 100%, 40%);">+ * according to TS 08.60 4.8.1 */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* once we have reached TRAU_FRAME_BITS, call</span><br><span style="color: hsl(120, 100%, 40%);">+ * the TRAU frame handler callback function */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sch->out_idx >= TRAU_FRAME_BITS) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sch->in_sync) {</span><br><span style="color: hsl(120, 100%, 40%);">+ dmx->out_cb(dmx, c, sch->out_bitbuf,</span><br><span style="color: hsl(120, 100%, 40%);">+ sch->out_idx, dmx->data);</span><br><span style="color: hsl(120, 100%, 40%);">+ sch->in_sync = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ sch->out_idx = 0;</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%);">+</span><br><span> /*! \brief Input some data from the 64k full-slot into subchannel demux</span><br><span> * \param[in] dmx subchannel demuxer</span><br><span> * \param[in] data pointer to buffer containing input data</span><br><span>@@ -108,7 +131,6 @@</span><br><span> uint8_t inbyte = data[i];</span><br><span> </span><br><span> for (c = 0; c < NR_SUBCH; c++) {</span><br><span style="color: hsl(0, 100%, 40%);">- struct demux_subch *sch = &dmx->subch[c];</span><br><span> uint8_t inbits;</span><br><span> uint8_t bit;</span><br><span> </span><br><span>@@ -123,33 +145,13 @@</span><br><span> bit = 1;</span><br><span> else</span><br><span> bit = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- append_bit(sch, bit);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (sync_hdr_complete(sch, bit))</span><br><span style="color: hsl(0, 100%, 40%);">- resync_to_here(sch);</span><br><span style="color: hsl(120, 100%, 40%);">+ append_bit_resync_out(dmx, c, bit);</span><br><span> </span><br><span> if (inbits & 0x02)</span><br><span> bit = 1;</span><br><span> else</span><br><span> bit = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- append_bit(sch, bit);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (sync_hdr_complete(sch, bit))</span><br><span style="color: hsl(0, 100%, 40%);">- resync_to_here(sch);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* FIXME: verify the first bit in octet 2, 4, 6, ...</span><br><span style="color: hsl(0, 100%, 40%);">- * according to TS 08.60 4.8.1 */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* once we have reached TRAU_FRAME_BITS, call</span><br><span style="color: hsl(0, 100%, 40%);">- * the TRAU frame handler callback function */</span><br><span style="color: hsl(0, 100%, 40%);">- if (sch->out_idx >= TRAU_FRAME_BITS) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (sch->in_sync) {</span><br><span style="color: hsl(0, 100%, 40%);">- dmx->out_cb(dmx, c, sch->out_bitbuf,</span><br><span style="color: hsl(0, 100%, 40%);">- sch->out_idx, dmx->data);</span><br><span style="color: hsl(0, 100%, 40%);">- sch->in_sync = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- sch->out_idx = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ append_bit_resync_out(dmx, c, bit);</span><br><span> }</span><br><span> }</span><br><span> return i;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmo-abis/+/19131">change 19131</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/c/libosmo-abis/+/19131"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmo-abis </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I58d946265372278051e4f29301d4f201ab98c0fc </div>
<div style="display:none"> Gerrit-Change-Number: 19131 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>