From patchwork Fri Oct 13 18:29:08 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sesterhenn X-Patchwork-Id: 825682 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yDGT73VDRz9s03 for ; Sat, 14 Oct 2017 05:29:15 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752845AbdJMS3O (ORCPT ); Fri, 13 Oct 2017 14:29:14 -0400 Received: from aibo.runbox.com ([91.220.196.211]:37510 "EHLO aibo.runbox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751981AbdJMS3N (ORCPT ); Fri, 13 Oct 2017 14:29:13 -0400 Received: from [10.9.9.210] (helo=mailfront10.runbox.com) by mailtransmit02.runbox with esmtp (Exim 4.86_2) (envelope-from ) id 1e34hq-0005jr-01; Fri, 13 Oct 2017 20:29:10 +0200 Received: from aftr-109-91-37-74.unity-media.net ([109.91.37.74] helo=[192.168.178.65]) by mailfront10.runbox.com with esmtpsa (uid:894066 ) (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.82) id 1e34hp-00006s-Mq; Fri, 13 Oct 2017 20:29:09 +0200 Subject: [PATCH] Bitwise Out Of Bound Read in Netfilter Conntrack To: Florian Westphal Cc: netfilter-devel@vger.kernel.org, pablo@netfilter.org References: <804512f5-786b-d4d0-bc8e-299c5c2683bf@x41-dsec.de> <20171012000346.GE26835@breakpoint.cc> From: Eric Sesterhenn Message-ID: <732a5fa7-1ccc-24f4-862c-18ff61b0fabb@x41-dsec.de> Date: Fri, 13 Oct 2017 20:29:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20171012000346.GE26835@breakpoint.cc> Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org From 66a5a55b6e7e45c51691583cf5833b4fe9365dc1 Mon Sep 17 00:00:00 2001 From: Eric Sesterhenn Date: Fri, 13 Oct 2017 20:26:32 +0200 Subject: [PATCH] Prevent bitwise out of bound memory reads The CHECK_BOUND function is not always uses, especially not when reading bitwise. Signed-off-by: Eric Sesterhenn --- net/netfilter/nf_conntrack_h323_asn1.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) CHECK_BOUND(bs, __tmp + 1); } }) static unsigned int get_len(bitstr_t *bs); static unsigned int get_bit(bitstr_t *bs); static unsigned int get_bits(bitstr_t *bs, unsigned int b); @@ -319,9 +320,11 @@ static int decode_int(bitstr_t *bs, const struct field_t *f, bs->cur += 2; break; case CONS: /* 64K < Range < 4G */ + CHECK_BIT_BOUND(bs, 2) len = get_bits(bs, 2) + 1; BYTE_ALIGN(bs); if (base && (f->attr & DECODE)) { /* timeToLive */ + CHECK_BOUND(bs, len); unsigned int v = get_uint(bs, len) + f->lb; PRINT(" = %u", v); *((unsigned int *)(base + f->offset)) = v; @@ -404,6 +407,7 @@ static int decode_numstr(bitstr_t *bs, const struct field_t *f, PRINT("%*.s%s\n", level * TAB_SIZE, " ", f->name); /* 2 <= Range <= 255 */ + CHECK_BIT_BOUND(bs, f->sz); len = get_bits(bs, f->sz) + f->lb; BYTE_ALIGN(bs); @@ -449,6 +453,7 @@ static int decode_octstr(bitstr_t *bs, const struct field_t *f, len = get_len(bs) + f->lb; break; default: /* 2 <= Range <= 255 */ + CHECK_BIT_BOUND(bs, f->sz); len = get_bits(bs, f->sz) + f->lb; BYTE_ALIGN(bs); break; @@ -477,6 +482,7 @@ static int decode_bmpstr(bitstr_t *bs, const struct field_t *f, len = (*bs->cur++) + f->lb; break; default: /* 2 <= Range <= 255 */ + CHECK_BIT_BOUND(bs, f->sz); len = get_bits(bs, f->sz) + f->lb; BYTE_ALIGN(bs); break; @@ -503,9 +509,11 @@ static int decode_seq(bitstr_t *bs, const struct field_t *f, base = (base && (f->attr & DECODE)) ? base + f->offset : NULL; /* Extensible? */ + CHECK_BIT_BOUND(bs, 1); ext = (f->attr & EXT) ? get_bit(bs) : 0; /* Get fields bitmap */ + CHECK_BIT_BOUND(bs, f->sz); bmp = get_bitmap(bs, f->sz); if (base) *(unsigned int *)base = bmp; @@ -555,8 +563,9 @@ static int decode_seq(bitstr_t *bs, const struct field_t *f, return H323_ERROR_NONE; /* Get the extension bitmap */ + CHECK_BIT_BOUND(bs, 7); bmp2_len = get_bits(bs, 7) + 1; - CHECK_BOUND(bs, (bmp2_len + 7) >> 3); + CHECK_BIT_BOUND(bs, bmp2_len); bmp2 = get_bitmap(bs, bmp2_len); bmp |= bmp2 >> f->sz; if (base) @@ -639,6 +648,7 @@ static int decode_seqof(bitstr_t *bs, const struct field_t *f, count = get_len(bs); break; default: + CHECK_BIT_BOUND(bs, f->sz); count = get_bits(bs, f->sz); break; } @@ -658,6 +668,7 @@ static int decode_seqof(bitstr_t *bs, const struct field_t *f, for (i = 0; i < count; i++) { if (son->attr & OPEN) { BYTE_ALIGN(bs); + CHECK_BOUND(bs, 2); len = get_len(bs); CHECK_BOUND(bs, len); if (!base || !(son->attr & DECODE)) { @@ -710,11 +721,14 @@ static int decode_choice(bitstr_t *bs, const struct field_t *f, base = (base && (f->attr & DECODE)) ? base + f->offset : NULL; /* Decode the choice index number */ + CHECK_BIT_BOUND(bs, 1); if ((f->attr & EXT) && get_bit(bs)) { ext = 1; + CHECK_BIT_BOUND(bs, 7); type = get_bits(bs, 7) + f->lb; } else { ext = 0; + CHECK_BIT_BOUND(bs, f->sz); type = get_bits(bs, f->sz); if (type >= f->lb) return H323_ERROR_RANGE; @@ -727,6 +741,7 @@ static int decode_choice(bitstr_t *bs, const struct field_t *f, /* Check Range */ if (type >= f->ub) { /* Newer version? */ BYTE_ALIGN(bs); + CHECK_BOUND(bs, 2); len = get_len(bs); CHECK_BOUND(bs, len); bs->cur += len; @@ -742,6 +757,7 @@ static int decode_choice(bitstr_t *bs, const struct field_t *f, if (ext || (son->attr & OPEN)) { BYTE_ALIGN(bs); + CHECK_BOUND(bs, 2); len = get_len(bs); CHECK_BOUND(bs, len); if (!base || !(son->attr & DECODE)) { diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c index 89b2e46925c4..e298878edc86 100644 --- a/net/netfilter/nf_conntrack_h323_asn1.c +++ b/net/netfilter/nf_conntrack_h323_asn1.c @@ -104,6 +104,7 @@ typedef struct { #define INC_BITS(bs,b) if(((bs)->bit+=(b))>7){(bs)->cur+=(bs)->bit>>3;(bs)->bit&=7;} #define BYTE_ALIGN(bs) if((bs)->bit){(bs)->cur++;(bs)->bit=0;} #define CHECK_BOUND(bs,n) if((bs)->cur+(n)>(bs)->end)return(H323_ERROR_BOUND) +#define CHECK_BIT_BOUND(bs,n) ({ size_t __tmp = n/8; if((bs)->bit+(n%8)>7) { CHECK_BOUND(bs, __tmp + 2); } else {