Skip to contentSkip to conversation

h323-conntrack-nat updates: buffer checks

PATCH In review Max Kellermann 2 participants 7 messages May 12, 2005 – May 17, 2005 netfilter

lore
Share & tools
lore

Max Kellermann

author
#0 lore raw

cover h323-conntrack-nat updates: buffer checks

Hi,

three small patches for the H.323 helper module. The first patch fixes a "nearly endless loop" bug, which I consider critical.

The three new patches depend on my other patches from the last two days.

Max

author
#1 lore raw

PATCH 1/3 h323-conntrack-nat updates: check bb->error in every iteration

h323-01-another_bb_error_check.patch
- critical bug fix, check bb->error in every for iteration

author
#2 lore raw

Re: [PATCH pom-ng 1/3] h323-conntrack-nat updates: check bb->error in every iteration

Quoted 3 lines · Max Kellermann

h323-01-another_bb_error_check.patch
- critical bug fix, check bb->error in every for iteration

damn, patch was missing :(

here it is

Thu May 12 21:58:49 CEST 2005 [email protected]
* check bb->error in loop

h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/asn1_per.c +1 -1
diff -rN -u old-h323-0/h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/asn1_per.c new-h323-0/h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/asn1_per.c
@@ -278,7 +278,7 @@
 	if (bb->error)
 		return;
 
-	for (i = 0; i < ext.count; i++) {
+	for (i = 0; i < ext.count && !bb->error; i++) {
 		if (asn1_per_bitmap_get(&ext.present, i))
 			asn1_per_skip_octet_string(bb);
 	}
author
#3 lore raw

PATCH 2/3 h323-conntrack-nat updates: q931_find_u2u() returns relative length

In reply to Max Kellermann

h323-02-u2u_length_is_relative.patch
- minor bugfix, never occurred in reality
- length returned by q931_find_u2u() is relative

author
#4 lore raw

Re: [PATCH pom-ng 2/3] h323-conntrack-nat updates: q931_find_u2u() returns relative length

Quoted 4 lines · Max Kellermann

h323-02-u2u_length_is_relative.patch
- minor bugfix, never occurred in reality
- length returned by q931_find_u2u() is relative

sorry, I'll never learn... ;)

Thu May 12 22:21:24 CEST 2005 [email protected]
* length returned by q931_find_u2u() is relative

h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/ip_conntrack_h323_h225.c +5 -3
diff -rN -u old-h323-0/h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/ip_conntrack_h323_h225.c new-h323-0/h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/ip_conntrack_h323_h225.c
@@ -214,9 +214,7 @@
 {
 	struct asn1_per_buffer bb;
 
-	/* protocol(1) + header(3) + protocolIdentifier(6) +
-	   h245ipAddress(1) + h245ipv4(4) + h245ipv4port(2) */
-	if (length < 17)
+	if (i + 8 > length)
 		return NF_ACCEPT;
 
 	if (data[i++] != 0x05) /* X.208 / X.209 */

@@ -297,6 +295,10 @@
 		if (i == 0)
 			return NF_ACCEPT;
 
+		/* the length returned by q931_find_u2u() is relative
+		   to i */
+		length += i;
+
 		return h225_parse_q931_connect(pskb, ct, ctinfo,
 					       data, i, length);
 	} else {
author
#5 lore raw

PATCH 3/3 h323-conntrack-nat updates: simplified length checks

In reply to Max Kellermann

h323-03-simplified_length_checks.patch
- simplified some length checks to make them easier readable

Thu May 12 22:38:17 CEST 2005 [email protected]
* simplified buffer length checks

h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/ip_conntrack_h323_h225.c +8 -5
diff -rN -u old-h323-0/h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/ip_conntrack_h323_h225.c new-h323-0/h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/ip_conntrack_h323_h225.c
@@ -214,7 +214,7 @@
 {
 	struct asn1_per_buffer bb;
 
-	if (i + 8 > length)
+	if (i + 2 > length)
 		return NF_ACCEPT;
 
 	if (data[i++] != 0x05) /* X.208 / X.209 */

@@ -275,6 +275,9 @@
 	u_int8_t q931_message_type;
 	unsigned length;
 
+	if (i + 3 > datalen)
+		return NF_ACCEPT;
+
 	/* parse Q.931 packet */
 	if (data[i++] != 0x08) /* protocol discriminator */
 		return NF_ACCEPT;

@@ -319,6 +322,9 @@
 	unsigned int i = 0;
 	u_int16_t tpkt_len;
 
+	if (i + 4 > datalen)
+		return NF_ACCEPT;
+
 	/* expect TPKT header, see RFC 1006 */
 	if (data[0] != 0x03 || data[1] != 0x00)
 		return NF_ACCEPT;

@@ -326,9 +332,6 @@
 	i += 2;
 
 	tpkt_len = ntohs(*(u_int16_t*)(data + i));
-	if (tpkt_len < 16)
-		return NF_ACCEPT;
-
 	if (tpkt_len < datalen)
 		datalen = tpkt_len;
 

@@ -372,7 +375,7 @@
 	}
 	datalen = (*pskb)->len - dataoff;
 
-	if (datalen < 32)
+	if (datalen < 16)
 		return NF_ACCEPT;
 
 	/* get data portion, and evaluate it */
h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/ip_conntrack_h323_h245.c +6 -3
diff -rN -u old-h323-0/h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/ip_conntrack_h323_h245.c new-h323-0/h323-conntrack-nat/linux-2.6.11/net/ipv4/netfilter/ip_conntrack_h323_h245.c
@@ -868,6 +868,9 @@
 	u_int16_t tpkt_len;
 	struct asn1_per_buffer bb;
 
+	if (i + 4 > datalen)
+		return NF_ACCEPT;
+
 	/* expect TPKT header, see RFC 1006 */
 	if (data[0] != 0x03 || data[1] != 0x00)
 		return NF_ACCEPT;

@@ -875,9 +878,6 @@
 	i += 2;
 
 	tpkt_len = ntohs(*(u_int16_t*)(data + i));
-	if (tpkt_len < 16)
-		return NF_ACCEPT;
-
 	if (tpkt_len < datalen)
 		datalen = tpkt_len;
 

@@ -922,6 +922,9 @@
 	}
 	datalen = (*pskb)->len - dataoff;
 
+	if (datalen < 16)
+		return NF_ACCEPT;
+
 	LOCK_BH(&ip_h245_lock);
 	data = skb_header_pointer((*pskb), dataoff,
 				  datalen, h245_buffer);
#6 lore raw

Re: [PATCH pom-ng 3/3] h323-conntrack-nat updates: simplified length checks

Quoted 2 lines · Max Kellermann

h323-03-simplified_length_checks.patch
- simplified some length checks to make them easier readable

All three patches applied, thanks.