* [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c @ 2010-03-20 14:32 wzt.wzt 2010-03-22 17:07 ` Patrick McHardy 0 siblings, 1 reply; 11+ messages in thread From: wzt.wzt @ 2010-03-20 14:32 UTC (permalink / raw) To: linux-kernel; +Cc: netfilter-devel, kaber The get.size field in the get_entries() interface is not bounded correctly. The size is used to determine the total entry size. The size is bounded, but can overflow and so the size checks may not be sufficient to catch invalid size. Fix it by catching size values that would cause overflows before calculating the size. Signed-off-by: Zhitong Wang <zhitong.wangzt@alibaba-inc.com> --- net/ipv4/netfilter/ip_tables.c | 4 ++++ net/ipv6/netfilter/ip6_tables.c | 4 ++++ 2 files changed, 8 insertions(+), 0 deletions(-) diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 4e7c719..6abd3d2 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -1164,6 +1164,10 @@ get_entries(struct net *net, struct ipt_get_entries __user *uptr, int *len) } if (copy_from_user(&get, uptr, sizeof(get)) != 0) return -EFAULT; + + if (get.size >= INT_MAX / sizeof(struct ipt_get_entries)) + return -EINVAL; + if (*len != sizeof(struct ipt_get_entries) + get.size) { duprintf("get_entries: %u != %zu\n", *len, sizeof(get) + get.size); diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c index 0b4557e..5185822 100644 --- a/net/ipv6/netfilter/ip6_tables.c +++ b/net/ipv6/netfilter/ip6_tables.c @@ -1190,6 +1190,10 @@ get_entries(struct net *net, struct ip6t_get_entries __user *uptr, int *len) } if (copy_from_user(&get, uptr, sizeof(get)) != 0) return -EFAULT; + + if (get.size >= INT_MAX / sizeof(struct ip6t_get_entries)) + return -EINVAL; + if (*len != sizeof(struct ip6t_get_entries) + get.size) { duprintf("get_entries: %u != %zu\n", *len, sizeof(get) + get.size); -- 1.6.5.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-20 14:32 [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c wzt.wzt @ 2010-03-22 17:07 ` Patrick McHardy 2010-03-23 1:34 ` wzt wzt 0 siblings, 1 reply; 11+ messages in thread From: Patrick McHardy @ 2010-03-22 17:07 UTC (permalink / raw) To: wzt.wzt; +Cc: linux-kernel, netfilter-devel wzt.wzt@gmail.com wrote: > The get.size field in the get_entries() interface is not bounded > correctly. The size is used to determine the total entry size. > The size is bounded, but can overflow and so the size checks may > not be sufficient to catch invalid size. Fix it by catching size > values that would cause overflows before calculating the size. > > Signed-off-by: Zhitong Wang <zhitong.wangzt@alibaba-inc.com> > > --- > net/ipv4/netfilter/ip_tables.c | 4 ++++ > net/ipv6/netfilter/ip6_tables.c | 4 ++++ > 2 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c > index 4e7c719..6abd3d2 100644 > --- a/net/ipv4/netfilter/ip_tables.c > +++ b/net/ipv4/netfilter/ip_tables.c > @@ -1164,6 +1164,10 @@ get_entries(struct net *net, struct ipt_get_entries __user *uptr, int *len) > } > if (copy_from_user(&get, uptr, sizeof(get)) != 0) > return -EFAULT; > + > + if (get.size >= INT_MAX / sizeof(struct ipt_get_entries)) > + return -EINVAL; I can see that the size might cause an overflow in the addition with sizeof(struct ipt_get_entries), but that would most likely cause a mismatch with the actual table size and get aborted (should be fixed anyways I guess). But I fail to find the overflow you're trying to prevent, which I guess would be the result of a multiplication. Please point me to the specific line in question. Thanks :) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-22 17:07 ` Patrick McHardy @ 2010-03-23 1:34 ` wzt wzt 2010-03-23 2:29 ` Xiaotian Feng 0 siblings, 1 reply; 11+ messages in thread From: wzt wzt @ 2010-03-23 1:34 UTC (permalink / raw) To: Patrick McHardy; +Cc: linux-kernel, netfilter-devel > I can see that the size might cause an overflow in the addition with > sizeof(struct ipt_get_entries) That's the integer overflow i pointed. get.size is copy from the user space, it can be set as 0x7fffffff, addition with sizeof(struct ipt_get_entries) can be overflow. if (*len != sizeof(struct ipt_get_entries) + get.size) { duprintf("get_entries: %u != %zu\n", *len, sizeof(get) + get.size); return -EINVAL; } so, check get.size max value before addition with sizeof(struct ipt_get_entries) to prevent the integer overflow. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-23 1:34 ` wzt wzt @ 2010-03-23 2:29 ` Xiaotian Feng 2010-03-23 2:37 ` wzt wzt 0 siblings, 1 reply; 11+ messages in thread From: Xiaotian Feng @ 2010-03-23 2:29 UTC (permalink / raw) To: wzt wzt; +Cc: Patrick McHardy, linux-kernel, netfilter-devel On Tue, Mar 23, 2010 at 9:34 AM, wzt wzt <wzt.wzt@gmail.com> wrote: >> I can see that the size might cause an overflow in the addition with >> sizeof(struct ipt_get_entries) > That's the integer overflow i pointed. > get.size is copy from the user space, it can be set as 0x7fffffff, > addition with sizeof(struct ipt_get_entries) can be overflow. Patrick's point is that you're using "if (get.size >= INT_MAX / sizeof(struct ipt_get_entries))" So, did you find any chance that get.size * sizeof(struct ipt_get_entries) >= INT_MAX ? And, for the addition overflow, can it be caught by "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? > > if (*len != sizeof(struct ipt_get_entries) + get.size) { > duprintf("get_entries: %u != %zu\n", > *len, sizeof(get) + get.size); > return -EINVAL; > } > > so, check get.size max value before addition with sizeof(struct > ipt_get_entries) to prevent the integer overflow. > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-23 2:29 ` Xiaotian Feng @ 2010-03-23 2:37 ` wzt wzt 2010-03-23 3:04 ` Jan Engelhardt 2010-03-23 3:05 ` Xiaotian Feng 0 siblings, 2 replies; 11+ messages in thread From: wzt wzt @ 2010-03-23 2:37 UTC (permalink / raw) To: Xiaotian Feng; +Cc: Patrick McHardy, linux-kernel, netfilter-devel > Patrick's point is that you're using "if (get.size >= INT_MAX / > sizeof(struct ipt_get_entries))" > So, did you find any chance that get.size * sizeof(struct > ipt_get_entries) >= INT_MAX ? > would you carefully read my explain??? get.size is copy from the user space, it can be set as 0x7fffffff, addition with sizeof(struct ipt_get_entries) can be overflow. > And, for the addition overflow, can it be caught by > > "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? > sizeof(struct ipt_get_entries) + get.size can be overflow as *len, get.size is control by user space with copy_from_user(). On Tue, Mar 23, 2010 at 10:29 AM, Xiaotian Feng <xtfeng@gmail.com> wrote: > On Tue, Mar 23, 2010 at 9:34 AM, wzt wzt <wzt.wzt@gmail.com> wrote: >>> I can see that the size might cause an overflow in the addition with >>> sizeof(struct ipt_get_entries) >> That's the integer overflow i pointed. >> get.size is copy from the user space, it can be set as 0x7fffffff, >> addition with sizeof(struct ipt_get_entries) can be overflow. > > Patrick's point is that you're using "if (get.size >= INT_MAX / > sizeof(struct ipt_get_entries))" > So, did you find any chance that get.size * sizeof(struct > ipt_get_entries) >= INT_MAX ? > > And, for the addition overflow, can it be caught by > > "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? > >> >> if (*len != sizeof(struct ipt_get_entries) + get.size) { >> duprintf("get_entries: %u != %zu\n", >> *len, sizeof(get) + get.size); >> return -EINVAL; >> } >> >> so, check get.size max value before addition with sizeof(struct >> ipt_get_entries) to prevent the integer overflow. >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ >> > -- 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-23 2:37 ` wzt wzt @ 2010-03-23 3:04 ` Jan Engelhardt 2010-03-23 3:48 ` wzt wzt 2010-03-23 3:05 ` Xiaotian Feng 1 sibling, 1 reply; 11+ messages in thread From: Jan Engelhardt @ 2010-03-23 3:04 UTC (permalink / raw) To: wzt wzt; +Cc: Xiaotian Feng, Patrick McHardy, linux-kernel, netfilter-devel On Tuesday 2010-03-23 03:37, wzt wzt wrote: >> And, for the addition overflow, can it be caught by >> >> "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? > >sizeof(struct ipt_get_entries) + get.size can be overflow as *len, >get.size is control by user space with copy_from_user(). The != should catch it. For 64-bit environments: * + invoked with size_t, unsigned int => right side promoted to size_t, result type is size_t * != invoked with int and size_t => left-side promoted to ssize_t (probably; but something as large as size_t) * get.size is 32-bit bounded, as is *len, so no overflow to worry about at all unless you make sizeof(X) hilariously big close to 2^64 which is rather unlikely. For 32-bit environments: * Let *len be a number of choice (e.g. 36) * Find a sizeof(X)+get.size that equals 36 mod 2^32. * Since sizeof(X) is const, get.size must be 0 mod 2^32. * So get.size must be a multiple of 2^32 to fool the system. * Since get.size itself is only a 32-bit quantity, you cannot represent any value larger than 4294967295. What Was What Was Wanted. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-23 3:04 ` Jan Engelhardt @ 2010-03-23 3:48 ` wzt wzt 2010-03-23 4:49 ` Xiaotian Feng 0 siblings, 1 reply; 11+ messages in thread From: wzt wzt @ 2010-03-23 3:48 UTC (permalink / raw) To: Jan Engelhardt Cc: Xiaotian Feng, Patrick McHardy, linux-kernel, netfilter-devel 1、 suppose *len = 35, sizeof(struct ipt_get_entries) = 36 2、 set get.size = 0xffffffff from user space 3、 sizeof(struct ipt_get_entries) + get.size = 36 + 0xffffffff = 35; 4、 if (*len != sizeof(struct ipt_get_entries) + get.size) was bypassed. you can test with c code: #include <stdio.h> int main(void) { unsigned int arg = 0xffffffff; printf("%u\n", arg + 36); if (35 != arg + 36) { printf("not over flow.\n"); return -1; } printf("arg over flow.\n"); return 0; } On Tue, Mar 23, 2010 at 11:04 AM, Jan Engelhardt <jengelh@medozas.de> wrote: > > On Tuesday 2010-03-23 03:37, wzt wzt wrote: >>> And, for the addition overflow, can it be caught by >>> >>> "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? >> >>sizeof(struct ipt_get_entries) + get.size can be overflow as *len, >>get.size is control by user space with copy_from_user(). > > The != should catch it. > > For 64-bit environments: > * + invoked with size_t, unsigned int > => right side promoted to size_t, result type is size_t > * != invoked with int and size_t > => left-side promoted to ssize_t (probably; but something as large as size_t) > * get.size is 32-bit bounded, as is *len, > so no overflow to worry about at all unless you make > sizeof(X) hilariously big close to 2^64 which is rather unlikely. > > For 32-bit environments: > * Let *len be a number of choice (e.g. 36) > * Find a sizeof(X)+get.size that equals 36 mod 2^32. > * Since sizeof(X) is const, get.size must be 0 mod 2^32. > * So get.size must be a multiple of 2^32 to fool the system. > * Since get.size itself is only a 32-bit quantity, you cannot > represent any value larger than 4294967295. > > > What Was What Was Wanted. > -- 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-23 3:48 ` wzt wzt @ 2010-03-23 4:49 ` Xiaotian Feng 2010-03-23 5:49 ` wzt wzt 0 siblings, 1 reply; 11+ messages in thread From: Xiaotian Feng @ 2010-03-23 4:49 UTC (permalink / raw) To: wzt wzt; +Cc: Jan Engelhardt, Patrick McHardy, linux-kernel, netfilter-devel On Tue, Mar 23, 2010 at 11:48 AM, wzt wzt <wzt.wzt@gmail.com> wrote: > 1、 suppose *len = 35, sizeof(struct ipt_get_entries) = 36 > 2、 set get.size = 0xffffffff from user space > 3、 sizeof(struct ipt_get_entries) + get.size = 36 + 0xffffffff = 35; > 4、 if (*len != sizeof(struct ipt_get_entries) + get.size) was bypassed. > > you can test with c code: > > #include <stdio.h> > > int main(void) > { > unsigned int arg = 0xffffffff; > > printf("%u\n", arg + 36); > if (35 != arg + 36) { > printf("not over flow.\n"); > return -1; > } > printf("arg over flow.\n"); > > return 0; > } > You didn't get his point... The key point is that sizeof() result type is size_t, slightly modify you code, try the result. > int main(void) > { > unsigned int arg = 0xffffffff; > unsigned int foo; > printf("%lu\n", arg + sizeof(foo)); > if (sizeof(foo) - 1 != arg + sizeof(foo)) { > printf("not over flow.\n"); > return -1; > } > printf("arg over flow.\n"); > > return 0; > } > On Tue, Mar 23, 2010 at 11:04 AM, Jan Engelhardt <jengelh@medozas.de> wrote: >> >> On Tuesday 2010-03-23 03:37, wzt wzt wrote: >>>> And, for the addition overflow, can it be caught by >>>> >>>> "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? >>> >>>sizeof(struct ipt_get_entries) + get.size can be overflow as *len, >>>get.size is control by user space with copy_from_user(). >> >> The != should catch it. >> >> For 64-bit environments: >> * + invoked with size_t, unsigned int >> => right side promoted to size_t, result type is size_t >> * != invoked with int and size_t >> => left-side promoted to ssize_t (probably; but something as large as size_t) >> * get.size is 32-bit bounded, as is *len, >> so no overflow to worry about at all unless you make >> sizeof(X) hilariously big close to 2^64 which is rather unlikely. >> >> For 32-bit environments: >> * Let *len be a number of choice (e.g. 36) >> * Find a sizeof(X)+get.size that equals 36 mod 2^32. >> * Since sizeof(X) is const, get.size must be 0 mod 2^32. >> * So get.size must be a multiple of 2^32 to fool the system. >> * Since get.size itself is only a 32-bit quantity, you cannot >> represent any value larger than 4294967295. >> >> >> What Was What Was Wanted. >> > -- 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-23 4:49 ` Xiaotian Feng @ 2010-03-23 5:49 ` wzt wzt 0 siblings, 0 replies; 11+ messages in thread From: wzt wzt @ 2010-03-23 5:49 UTC (permalink / raw) To: Xiaotian Feng Cc: Jan Engelhardt, Patrick McHardy, linux-kernel, netfilter-devel > You didn't get his point... The key point is that sizeof() result type > is size_t, slightly modify you code, try the result. >> int main(void) >> { >> unsigned int arg = 0xffffffff; >> unsigned int foo; >> printf("%lu\n", arg + sizeof(foo)); >> if (sizeof(foo) - 1 != arg + sizeof(foo)) { >> printf("not over flow.\n"); >> return -1; >> } >> printf("arg over flow.\n"); >> >> return 0; >> } sizeof() is size_t(unsigned int), get.size is also unsigned int, arg + sizeof(foo) can overflow as sizeof(foo) - 1 [root@localhost test]# ./test 3 arg over flow. my fault is *len is check with: if (*len < sizeof(get)) { duprintf("get_entries: %u < %zu\n", *len, sizeof(get)); return -EINVAL; } so, *len must >= sizeof(get), then: if (*len != sizeof(struct ipt_get_entries) + get.size) { duprintf("get_entries: %u != %zu\n", *len, sizeof(get) + get.size); return -EINVAL; } if sizeof(struct ipt_get_entries) + get.size oveflow, it must < sizeof(struct ipt_get_entries), It not make any problem, just return. get.size overflow can't make any problem, thx Feng and Jan for helping me. On Tue, Mar 23, 2010 at 12:49 PM, Xiaotian Feng <xtfeng@gmail.com> wrote: > On Tue, Mar 23, 2010 at 11:48 AM, wzt wzt <wzt.wzt@gmail.com> wrote: >> 1、 suppose *len = 35, sizeof(struct ipt_get_entries) = 36 >> 2、 set get.size = 0xffffffff from user space >> 3、 sizeof(struct ipt_get_entries) + get.size = 36 + 0xffffffff = 35; >> 4、 if (*len != sizeof(struct ipt_get_entries) + get.size) was bypassed. >> >> you can test with c code: >> >> #include <stdio.h> >> >> int main(void) >> { >> unsigned int arg = 0xffffffff; >> >> printf("%u\n", arg + 36); >> if (35 != arg + 36) { >> printf("not over flow.\n"); >> return -1; >> } >> printf("arg over flow.\n"); >> >> return 0; >> } >> > > You didn't get his point... The key point is that sizeof() result type > is size_t, slightly modify you code, try the result. >> int main(void) >> { >> unsigned int arg = 0xffffffff; >> unsigned int foo; >> printf("%lu\n", arg + sizeof(foo)); >> if (sizeof(foo) - 1 != arg + sizeof(foo)) { >> printf("not over flow.\n"); >> return -1; >> } >> printf("arg over flow.\n"); >> >> return 0; >> } > >> On Tue, Mar 23, 2010 at 11:04 AM, Jan Engelhardt <jengelh@medozas.de> wrote: >>> >>> On Tuesday 2010-03-23 03:37, wzt wzt wrote: >>>>> And, for the addition overflow, can it be caught by >>>>> >>>>> "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? >>>> >>>>sizeof(struct ipt_get_entries) + get.size can be overflow as *len, >>>>get.size is control by user space with copy_from_user(). >>> >>> The != should catch it. >>> >>> For 64-bit environments: >>> * + invoked with size_t, unsigned int >>> => right side promoted to size_t, result type is size_t >>> * != invoked with int and size_t >>> => left-side promoted to ssize_t (probably; but something as large as size_t) >>> * get.size is 32-bit bounded, as is *len, >>> so no overflow to worry about at all unless you make >>> sizeof(X) hilariously big close to 2^64 which is rather unlikely. >>> >>> For 32-bit environments: >>> * Let *len be a number of choice (e.g. 36) >>> * Find a sizeof(X)+get.size that equals 36 mod 2^32. >>> * Since sizeof(X) is const, get.size must be 0 mod 2^32. >>> * So get.size must be a multiple of 2^32 to fool the system. >>> * Since get.size itself is only a 32-bit quantity, you cannot >>> represent any value larger than 4294967295. >>> >>> >>> What Was What Was Wanted. >>> >> > -- 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-23 2:37 ` wzt wzt 2010-03-23 3:04 ` Jan Engelhardt @ 2010-03-23 3:05 ` Xiaotian Feng 2010-03-23 3:11 ` wzt wzt 1 sibling, 1 reply; 11+ messages in thread From: Xiaotian Feng @ 2010-03-23 3:05 UTC (permalink / raw) To: wzt wzt; +Cc: Patrick McHardy, linux-kernel, netfilter-devel On Tue, Mar 23, 2010 at 10:37 AM, wzt wzt <wzt.wzt@gmail.com> wrote: >> Patrick's point is that you're using "if (get.size >= INT_MAX / >> sizeof(struct ipt_get_entries))" >> So, did you find any chance that get.size * sizeof(struct >> ipt_get_entries) >= INT_MAX ? >> > would you carefully read my explain??? > get.size is copy from the user space, it can be set as 0x7fffffff, > addition with sizeof(struct ipt_get_entries) can be overflow. > get.size is unsigned int, UINT_MAX is 0x FFFFFFFF, not 0x7FFFFFFF And you're metioning "addition", then why you're checking as "multiplication"? >> And, for the addition overflow, can it be caught by >> >> "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? >> > sizeof(struct ipt_get_entries) + get.size can be overflow as *len, > get.size is control by user space with copy_from_user(). > > On Tue, Mar 23, 2010 at 10:29 AM, Xiaotian Feng <xtfeng@gmail.com> wrote: >> On Tue, Mar 23, 2010 at 9:34 AM, wzt wzt <wzt.wzt@gmail.com> wrote: >>>> I can see that the size might cause an overflow in the addition with >>>> sizeof(struct ipt_get_entries) >>> That's the integer overflow i pointed. >>> get.size is copy from the user space, it can be set as 0x7fffffff, >>> addition with sizeof(struct ipt_get_entries) can be overflow. >> >> Patrick's point is that you're using "if (get.size >= INT_MAX / >> sizeof(struct ipt_get_entries))" >> So, did you find any chance that get.size * sizeof(struct >> ipt_get_entries) >= INT_MAX ? >> >> And, for the addition overflow, can it be caught by >> >> "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? >> >>> >>> if (*len != sizeof(struct ipt_get_entries) + get.size) { >>> duprintf("get_entries: %u != %zu\n", >>> *len, sizeof(get) + get.size); >>> return -EINVAL; >>> } >>> >>> so, check get.size max value before addition with sizeof(struct >>> ipt_get_entries) to prevent the integer overflow. >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> Please read the FAQ at http://www.tux.org/lkml/ >>> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c 2010-03-23 3:05 ` Xiaotian Feng @ 2010-03-23 3:11 ` wzt wzt 0 siblings, 0 replies; 11+ messages in thread From: wzt wzt @ 2010-03-23 3:11 UTC (permalink / raw) To: Xiaotian Feng; +Cc: Patrick McHardy, linux-kernel, netfilter-devel > get.size is unsigned int, UINT_MAX is 0x FFFFFFFF, not 0x7FFFFFFF > And you're metioning "addition", then why you're checking as "multiplication"? oh, my falut:( the patch is multiplication check, not addition check. thanks for helping me:) On Tue, Mar 23, 2010 at 11:05 AM, Xiaotian Feng <xtfeng@gmail.com> wrote: > On Tue, Mar 23, 2010 at 10:37 AM, wzt wzt <wzt.wzt@gmail.com> wrote: >>> Patrick's point is that you're using "if (get.size >= INT_MAX / >>> sizeof(struct ipt_get_entries))" >>> So, did you find any chance that get.size * sizeof(struct >>> ipt_get_entries) >= INT_MAX ? >>> >> would you carefully read my explain??? >> get.size is copy from the user space, it can be set as 0x7fffffff, >> addition with sizeof(struct ipt_get_entries) can be overflow. >> > > get.size is unsigned int, UINT_MAX is 0x FFFFFFFF, not 0x7FFFFFFF > And you're metioning "addition", then why you're checking as "multiplication"? > >>> And, for the addition overflow, can it be caught by >>> >>> "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? >>> >> sizeof(struct ipt_get_entries) + get.size can be overflow as *len, >> get.size is control by user space with copy_from_user(). > > >> >> On Tue, Mar 23, 2010 at 10:29 AM, Xiaotian Feng <xtfeng@gmail.com> wrote: >>> On Tue, Mar 23, 2010 at 9:34 AM, wzt wzt <wzt.wzt@gmail.com> wrote: >>>>> I can see that the size might cause an overflow in the addition with >>>>> sizeof(struct ipt_get_entries) >>>> That's the integer overflow i pointed. >>>> get.size is copy from the user space, it can be set as 0x7fffffff, >>>> addition with sizeof(struct ipt_get_entries) can be overflow. >>> >>> Patrick's point is that you're using "if (get.size >= INT_MAX / >>> sizeof(struct ipt_get_entries))" >>> So, did you find any chance that get.size * sizeof(struct >>> ipt_get_entries) >= INT_MAX ? >>> >>> And, for the addition overflow, can it be caught by >>> >>> "if (*len != sizeof(struct ipt_get_entries) + get.size)" ??? >>> >>>> >>>> if (*len != sizeof(struct ipt_get_entries) + get.size) { >>>> duprintf("get_entries: %u != %zu\n", >>>> *len, sizeof(get) + get.size); >>>> return -EINVAL; >>>> } >>>> >>>> so, check get.size max value before addition with sizeof(struct >>>> ipt_get_entries) to prevent the integer overflow. >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> Please read the FAQ at http://www.tux.org/lkml/ >>>> >>> >> > -- 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-03-23 5:49 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-03-20 14:32 [PATCH] Netfilter: Fix integer overflow in net/ipv6/netfilter/ip6_tables.c wzt.wzt 2010-03-22 17:07 ` Patrick McHardy 2010-03-23 1:34 ` wzt wzt 2010-03-23 2:29 ` Xiaotian Feng 2010-03-23 2:37 ` wzt wzt 2010-03-23 3:04 ` Jan Engelhardt 2010-03-23 3:48 ` wzt wzt 2010-03-23 4:49 ` Xiaotian Feng 2010-03-23 5:49 ` wzt wzt 2010-03-23 3:05 ` Xiaotian Feng 2010-03-23 3:11 ` wzt wzt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).