From patchwork Mon Jun 23 07:50:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Harald Welte X-Patchwork-Id: 362685 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ganesha.gnumonks.org (ganesha.gnumonks.org [IPv6:2001:780:45:1d:225:90ff:fe52:c662]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 248C5140094 for ; Mon, 23 Jun 2014 17:51:42 +1000 (EST) Received: from localhost ([127.0.0.1] helo=ganesha.gnumonks.org) by ganesha.gnumonks.org with esmtp (Exim 4.72) (envelope-from ) id 1Wyz2F-0008Ft-SS; Mon, 23 Jun 2014 09:51:28 +0200 Received: from uucp by ganesha.gnumonks.org with local-bsmtp (Exim 4.72) (envelope-from ) id 1Wyz1r-0008Fd-QS; Mon, 23 Jun 2014 09:51:04 +0200 Received: from laforge by localhost.localdomain with local (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1Wyz1c-0000gV-4p; Mon, 23 Jun 2014 09:50:48 +0200 Date: Mon, 23 Jun 2014 09:50:48 +0200 From: Harald Welte To: Holger Hans Peter Freyther Subject: Re: trau_decode_fr memory corruption. Fix requested Message-ID: <20140623075048.GC5903@nataraja> References: <20140528145811.GA6793@xiaoyu.lan> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140528145811.GA6793@xiaoyu.lan> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: openbsc@lists.osmocom.org, andreas@eversberg.eu X-BeenThere: openbsc@lists.osmocom.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Development of the OpenBSC GSM base station controller List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: openbsc-bounces@lists.osmocom.org Errors-To: openbsc-bounces@lists.osmocom.org 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 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 --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;