Hi Eric, Is there any change with regards to previous patch? I have two in patchwork: http://patchwork.ozlabs.org/patch/823074/ http://patchwork.ozlabs.org/patch/825682/ Which one should I consider for submission? Thanks! On Fri, Oct 13, 2017 at 08:29:08PM +0200, Eric Sesterhenn wrote: > > From 66a5a55b6e7e45c51691583cf5833b4fe9365dc1 Mon Sep 17 00:00:00 2001 > From: Eric Sesterhenn <eric.sesterhenn@xxxxxxxxxxx> > 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 <eric.sesterhenn@xxxxxxxxxxx> > --- > net/netfilter/nf_conntrack_h323_asn1.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > 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 { > 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)) { > > > -- > 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 > -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
