diff mbox

trau_decode_fr memory corruption. Fix requested

Message ID 20140623075048.GC5903@nataraja
State Accepted
Headers show

Commit Message

Harald Welte June 23, 2014, 7:50 a.m. UTC 
Hi Holger,

On Wed, May 28, 2014 at 04:58:11PM +0200, Holger Hans Peter Freyther wrote:
> I don't really know much about the bit order of TRAU frames but
> the trau_test.c is causing an out of bounds access to the gsm_fr_map.

It is the last iteration of the loop, where i==259 and o==260.  It is
read out-of-bounds but the content is never used.  So yes, it is an
out-of-bounds access, but one that's unlikely to cause any problems
[unless the end of the 'gsm_fr_map' is the edge of the address space]

The only way I can think to avoid it is by putting additional
conditionals in the code, which might have performance implications:

Fixed in git:


From 9f109dfb9926558b6ea504dc3aee92cfd64413bd Mon Sep 17 00:00:00 2001
From: Harald Welte <laforge@gnumonks.org>
Date: Mon, 23 Jun 2014 09:48:07 +0200
Subject: [PATCH 1/2] trau_mux.c: Prevent out-of-bounds read in
 trau_encode_fr()

found by -fsanitize=address the last iteration of the loop, where i ==
259 and o == 260.  It is read out-of-bounds but the content is never
used.
---
 openbsc/src/libtrau/trau_mux.c | 3 +++
 1 file changed, 3 insertions(+)
diff mbox

Patch

diff --git a/openbsc/src/libtrau/trau_mux.c b/openbsc/src/libtrau/trau_mux.c
index fd1895f..4f159e4 100644
--- a/openbsc/src/libtrau/trau_mux.c
+++ b/openbsc/src/libtrau/trau_mux.c
@@ -436,6 +436,9 @@  void trau_encode_fr(struct decoded_trau_frame *tf,
 	o = 0; /* offset output bits */
 	while (i < 260) {
 		tf->d_bits[k+o] = (data[j/8] >> (7-(j%8))) & 1;
+		/* to avoid out-of-bounds access in gsm_fr_map[++l] */
+		if (i == 259)
+			break;
 		if (--k < 0) {
 			o += gsm_fr_map[l];
 			k = gsm_fr_map[++l]-1;