| Message ID | CAML0wpEBwXdzWX7g3LDJRDOVM0J7dkuhH8b+96ZhRf54+qUgwQ@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
Toby DiPasquale <toby@cbcg.net> wrote: > I was reviewing the H.323 conntrack helper in the kernel when I came > across what appears to be an off-by-one error in the DecodeQ931 > function. The MessageType field of the Q931 record is assigned and p > is incremented, but the corresponding decrement to sz is missing, > leading the sz variable to be one more than it should be. This patch > decrements sz so it is the proper value going into the parsing of the > information elements. > > Signed-off-by: Toby DiPasquale <toby@cbcg.net> Looks correct, BUT > diff --git a/net/netfilter/nf_conntrack_h323_asn1.c > b/net/netfilter/nf_conntrack_h323_asn1.c > index bcd5ed6..68b1557 100644 > --- a/net/netfilter/nf_conntrack_h323_asn1.c > +++ b/net/netfilter/nf_conntrack_h323_asn1.c > @@ -849,6 +849,7 @@ int DecodeQ931(unsigned char *buf, size_t sz, Q931 *q931) > if (sz < 1) > return H323_ERROR_BOUND; sz can be 1 > q931->MessageType = *p++; > + sz--; sz is now 0 > PRINT("MessageType = %02X\n", q931->MessageType); > if (*p & 0x80) { > p++; > sz--; -> sz (size_t) will underflow here I'd suggest to change the if (sz < 1) to if (sz < 2) to resolve this, the while loop below has to be taken anyway. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c index bcd5ed6..68b1557 100644 --- a/net/netfilter/nf_conntrack_h323_asn1.c +++ b/net/netfilter/nf_conntrack_h323_asn1.c @@ -849,6 +849,7 @@ int DecodeQ931(unsigned char *buf, size_t sz, Q931 *q931) if (sz < 1) return H323_ERROR_BOUND; q931->MessageType = *p++; + sz--; PRINT("MessageType = %02X\n", q931->MessageType); if (*p & 0x80) { p++;
I was reviewing the H.323 conntrack helper in the kernel when I came across what appears to be an off-by-one error in the DecodeQ931 function. The MessageType field of the Q931 record is assigned and p is incremented, but the corresponding decrement to sz is missing, leading the sz variable to be one more than it should be. This patch decrements sz so it is the proper value going into the parsing of the information elements. Signed-off-by: Toby DiPasquale <toby@cbcg.net> --