Hi Pablo, On 24/10/17 18:36, Pablo Neira Ayuso wrote: > On Tue, Oct 24, 2017 at 06:29:03PM +0200, Pablo Neira Ayuso wrote: >> Hi, >> >> On Mon, Oct 09, 2017 at 07:01:14AM +0200, Eric Sesterhenn wrote: >>> Add missing counter decrement to prevent out of bounds memory read. >> >> I have manually applied this. However, your patches are mangled most >> likely by MUA and patchword doesn't get it right. >> >> If you could please revamp and make sure patches apply cleanly via: >> >> git am patch >> >> I would really appreciate, thanks for your patience! > I tried to attach the patches in order to keep thunderbird from mangling them. Guess its time to set up a second mua for mailing patches. > While at it, please prepend to your patches: > > netfilter: nf_ct_h323: ... > > So we all know what subsystems this goes to. As in the patches attached? Regards, Eric -- Eric Sesterhenn (Principal Security Consultant) X41 D-SEC GmbH, Dennewartstr. 25-27, D-52068 Aachen T: +49 241 9809418-0, Fax: -9 Unternehmenssitz: Aachen, Amtsgericht Aachen: HRB19989 Geschäftsführer: Markus Vervier
From b8ed8753ca82f6f07fce2901418aab531d98ee39 Mon Sep 17 00:00:00 2001 From: Eric Sesterhenn <eric.sesterhenn@xxxxxxxxxxx> Date: Wed, 25 Oct 2017 08:32:57 +0200 Subject: [PATCH netfilter: nf_ct_h323: 1/2] Out Of Bound Read in Netfilter Conntrack Add missing counter decrement to prevent out of bounds memory read. Signed-off-by: Eric Sesterhenn <eric.sesterhenn@xxxxxxxxxxx> --- net/netfilter/nf_conntrack_h323_asn1.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c index 89b2e46925c4..2a9d1acd0cbd 100644 --- a/net/netfilter/nf_conntrack_h323_asn1.c +++ b/net/netfilter/nf_conntrack_h323_asn1.c @@ -877,6 +877,7 @@ int DecodeQ931(unsigned char *buf, size_t sz, Q931 *q931) if (sz < 1) break; len = *p++; + sz--; if (sz < len) break; p += len; -- 2.11.0
From c1b7044749e534207ecd3b04281ae024b01887d3 Mon Sep 17 00:00:00 2001
From: Eric Sesterhenn <eric.sesterhenn@xxxxxxxxxxx>
Date: Wed, 25 Oct 2017 08:39:38 +0200
Subject: [PATCH netfilter: nf_ct_h323: 2/2] Prevent multiple out of bounds
memory reads.
Multiple accesses are not guarded by out of bound
checks. This patch introduces them.
Signed-off-by: Eric Sesterhenn <eric.sesterhenn@xxxxxxxxxxx>
---
net/netfilter/nf_conntrack_h323_asn1.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
index 2a9d1acd0cbd..78a218cdf04e 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 { 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,6 +320,7 @@ 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 */
@@ -404,6 +406,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 +452,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 +481,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 +508,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 +562,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 +647,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 +667,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 +720,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 +740,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 +756,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)) {
--
2.11.0
Attachment:
signature.asc
Description: OpenPGP digital signature
