[openbsc 3/3] libtrau: Fix off-by-one read access to gsm_fr_map

[Daniel Willmann]

Address sanitizer complains with a buffer overflow to the end of
gsm_fr_map:

ERROR: AddressSanitizer: global-buffer-overflow on address 0x00000044f76c at pc 0x43c0dd bp 0x7fff18389db0 sp 0x7fff18389da8
READ of size 1 at 0x00000044f76c thread T0
    #0 0x43c0dc in trau_encode_fr /home/alphaone/scm/osmo/openbsc/openbsc/src/libtrau/trau_mux.c:441
    #1 0x42fad6 in test_trau_fr_efr /home/alphaone/scm/osmo/openbsc/openbsc/tests/trau/trau_test.c:35
    #2 0x4308f4 in main /home/alphaone/scm/osmo/openbsc/openbsc/tests/trau/trau_test.c:70
    #3 0x7f96e8cf04bc (/lib64/libc.so.6+0x224bc)
    #4 0x42f7ec (/home/alphaone/scm/osmo/openbsc/openbsc/tests/trau/trau_test+0x42f7ec)
0x00000044f76c is located 52 bytes to the left of global variable 'c_bits_check_fr' from 'trau_mux.c' (0x44f7a0) of size 5
0x00000044f76c is located 0 bytes to the right of global variable 'gsm_fr_map' from 'trau_mux.c' (0x44f720) of size 76
SUMMARY: AddressSanitizer: global-buffer-overflow /home/alphaone/scm/osmo/openbsc/openbsc/src/libtrau/trau_mux.c:441 trau_encode_fr

In the last iteration of the loop k is already set to the next element
in gsm_fr_map which leads to an out-of-bounds read. Instead decrement k
at the end of the loop and put the check before the data assignment.
This is functionally equivalent as k is never < 0 initially.

This happens in trau_decode_fr as well.
---
 openbsc/src/libtrau/trau_mux.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/openbsc/src/libtrau/trau_mux.c b/openbsc/src/libtrau/trau_mux.c
index 4f159e4..9b93eda 100644
--- a/openbsc/src/libtrau/trau_mux.c
+++ b/openbsc/src/libtrau/trau_mux.c
@@ -234,13 +234,14 @@ struct msgb *trau_decode_fr(uint32_t callref,
 	l = 0; /* counts element bits */
 	o = 0; /* offset input bits */
 	while (i < 260) {
-		data[j/8] |= (tf->d_bits[k+o] << (7-(j%8)));
-		if (--k < 0) {
+		if (k < 0) {
 			o += gsm_fr_map[l];
 			k = gsm_fr_map[++l]-1;
 		}
+		data[j/8] |= (tf->d_bits[k+o] << (7-(j%8)));
 		i++;
 		j++;
+		k--;
 	}
 	frame->msg_type = GSM_TCHF_FRAME;
 	frame->callref = callref;
@@ -435,16 +436,14 @@ void trau_encode_fr(struct decoded_trau_frame *tf,
 	l = 0; /* counts element bits */
 	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) {
+		if (k < 0) {
 			o += gsm_fr_map[l];
 			k = gsm_fr_map[++l]-1;
 		}
+		tf->d_bits[k+o] = (data[j/8] >> (7-(j%8))) & 1;
 		i++;
 		j++;
+		k--;
 	}
 }
 
-- 
1.8.4.2