Change in libosmo-abis[master]: subchan_demux: Fix out-of-bounds write

[laforge]

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-abis/+/19131 )


Change subject: subchan_demux: Fix out-of-bounds write
......................................................................

subchan_demux: Fix out-of-bounds write

We cannot blindly append two ubits to the 320-ubit sized buffer.  In the
end, we may already fill the buffer after the first ubit, causing a
buffer overflow with the second ubit.

Lets check if the buffer is full after every bit.  Avoid copy+pasting
but move the code repeated per bit to a new function.

Change-Id: I58d946265372278051e4f29301d4f201ab98c0fc
Closes: OS#4648
---
M src/subchan_demux.c
1 file changed, 25 insertions(+), 23 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/31/19131/1

diff --git a/src/subchan_demux.c b/src/subchan_demux.c
index 55503db..a3a44d9 100644
--- a/src/subchan_demux.c
+++ b/src/subchan_demux.c
@@ -92,6 +92,29 @@
 	return 0;
 }
 
+static void append_bit_resync_out(struct subch_demux *dmx, int c, ubit_t bit)
+{
+	struct demux_subch *sch = &dmx->subch[c];
+	append_bit(sch, bit);
+
+	if (sync_hdr_complete(sch, bit))
+		resync_to_here(sch);
+
+	/* FIXME: verify the first bit in octet 2, 4, 6, ...
+	 * according to TS 08.60 4.8.1 */
+
+	/* once we have reached TRAU_FRAME_BITS, call
+	 * the TRAU frame handler callback function */
+	if (sch->out_idx >= TRAU_FRAME_BITS) {
+		if (sch->in_sync) {
+			dmx->out_cb(dmx, c, sch->out_bitbuf,
+			    sch->out_idx, dmx->data);
+			sch->in_sync = 0;
+		}
+		sch->out_idx = 0;
+	}
+}
+
 /*! \brief Input some data from the 64k full-slot into subchannel demux
  *  \param[in] dmx subchannel demuxer
  *  \param[in] data pointer to buffer containing input data
@@ -108,7 +131,6 @@
 		uint8_t inbyte = data[i];
 
 		for (c = 0; c < NR_SUBCH; c++) {
-			struct demux_subch *sch = &dmx->subch[c];
 			uint8_t inbits;
 			uint8_t bit;
 
@@ -123,33 +145,13 @@
 				bit = 1;
 			else
 				bit = 0;
-			append_bit(sch, bit);
-
-			if (sync_hdr_complete(sch, bit))
-				resync_to_here(sch);
+			append_bit_resync_out(dmx, c, bit);
 
 			if (inbits & 0x02)
 				bit = 1;
 			else
 				bit = 0;
-			append_bit(sch, bit);
-
-			if (sync_hdr_complete(sch, bit))
-				resync_to_here(sch);
-
-			/* FIXME: verify the first bit in octet 2, 4, 6, ...
-			 * according to TS 08.60 4.8.1 */
-
-			/* once we have reached TRAU_FRAME_BITS, call
-			 * the TRAU frame handler callback function */
-			if (sch->out_idx >= TRAU_FRAME_BITS) {
-				if (sch->in_sync) {
-					dmx->out_cb(dmx, c, sch->out_bitbuf,
-					    sch->out_idx, dmx->data);
-					sch->in_sync = 0;
-				}
-				sch->out_idx = 0;
-			}
+			append_bit_resync_out(dmx, c, bit);
 		}
 	}
 	return i;

-- 
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/19131
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I58d946265372278051e4f29301d4f201ab98c0fc
Gerrit-Change-Number: 19131
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200704/123f4f49/attachment.htm>