Re: [netfilter-core] Heap overflow in xt_geoip.c

Re: [netfilter-core] Heap overflow in xt_geoip.c

[Jan Engelhardt]

On Wednesday 2017-06-21 18:16, Pablo Neira Ayuso wrote:

>Hi Loic,
>
>On Tue, Jun 20, 2017 at 08:31:26PM +0200, Loic wrote:
>> Hi,
>> 
>> I think there is a problem in the geoip code because I detect this:
>> 
>> grep -ar "cicus.162_313 max" /usr/src/xtables-addons-2.12/extensions/
>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.o:cicus.162_313 max,
>> count: 7, decl: vmalloc; num: 1; context: fndecl;
>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.o:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313
>> max, count: 5, decl: size_overflow MARK_NO copy_user_generic 3; num:
>> 0; context: attr;
>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.ko:cicus.162_313 max,
>> count: 7, decl: vmalloc; num: 1; context: fndecl;
>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.ko:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313
>> max, count: 5, decl: size_overflow MARK_NO copy_user_generic 3; num:
>> 0; context: attr;
>> 
>> You maybe can draw inspiration for resolve this by "vmalloc_usercopy" in
>> PAX_USERCOPY from PaX/Grsecurity.
>
>This is out of tree code, Cc'ing Jan, who maintains this.

What is cicus and what are these messages supposed to tell me?

Re: [netfilter-core] Heap overflow in xt_geoip.c

[Loic]

Le 2017-06-25 21:45, Jan Engelhardt a écrit :
> On Wednesday 2017-06-21 18:16, Pablo Neira Ayuso wrote:
> 
>> Hi Loic,
>> 
>> On Tue, Jun 20, 2017 at 08:31:26PM +0200, Loic wrote:
>>> Hi,
>>> 
>>> I think there is a problem in the geoip code because I detect this:
>>> 
>>> grep -ar "cicus.162_313 max" /usr/src/xtables-addons-2.12/extensions/
>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.o:cicus.162_313 max,
>>> count: 7, decl: vmalloc; num: 1; context: fndecl;
>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.o:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313
>>> max, count: 5, decl: size_overflow MARK_NO copy_user_generic 3; num:
>>> 0; context: attr;
>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.ko:cicus.162_313 
>>> max,
>>> count: 7, decl: vmalloc; num: 1; context: fndecl;
>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.ko:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313
>>> max, count: 5, decl: size_overflow MARK_NO copy_user_generic 3; num:
>>> 0; context: attr;
>>> 
>>> You maybe can draw inspiration for resolve this by "vmalloc_usercopy" 
>>> in
>>> PAX_USERCOPY from PaX/Grsecurity.
>> 
>> This is out of tree code, Cc'ing Jan, who maintains this.
> 
> What is cicus and what are these messages supposed to tell me?

This comes from the size_overflow plugin :
https://github.com/ephox-gcc-plugins/size_overflow

After I'm not an expert I just detected this:
grep -ai size_overflow "xt_geoip.ko"
/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313 max, 
count: 5, decl: # size_overflow MARK_NO copy_user_generic 3; num: 0; 
context: attr;

-- 
Best regards,

Loic

Re: [netfilter-core] Heap overflow in xt_geoip.c

[Loic]

Le 2017-06-26 20:41, Loic a écrit :
> Le 2017-06-25 21:45, Jan Engelhardt a écrit :
>> On Wednesday 2017-06-21 18:16, Pablo Neira Ayuso wrote:
>> 
>>> Hi Loic,
>>> 
>>> On Tue, Jun 20, 2017 at 08:31:26PM +0200, Loic wrote:
>>>> Hi,
>>>> 
>>>> I think there is a problem in the geoip code because I detect this:
>>>> 
>>>> grep -ar "cicus.162_313 max" 
>>>> /usr/src/xtables-addons-2.12/extensions/
>>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.o:cicus.162_313 
>>>> max,
>>>> count: 7, decl: vmalloc; num: 1; context: fndecl;
>>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.o:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313
>>>> max, count: 5, decl: size_overflow MARK_NO copy_user_generic 3; num:
>>>> 0; context: attr;
>>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.ko:cicus.162_313 
>>>> max,
>>>> count: 7, decl: vmalloc; num: 1; context: fndecl;
>>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.ko:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313
>>>> max, count: 5, decl: size_overflow MARK_NO copy_user_generic 3; num:
>>>> 0; context: attr;
>>>> 
>>>> You maybe can draw inspiration for resolve this by 
>>>> "vmalloc_usercopy" in
>>>> PAX_USERCOPY from PaX/Grsecurity.
>>> 
>>> This is out of tree code, Cc'ing Jan, who maintains this.
>> 
>> What is cicus and what are these messages supposed to tell me?
> 
> This comes from the size_overflow plugin :
> https://github.com/ephox-gcc-plugins/size_overflow
> 
> After I'm not an expert I just detected this:
> grep -ai size_overflow "xt_geoip.ko"
> /usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313 max,
> count: 5, decl: # size_overflow MARK_NO copy_user_generic 3; num: 0;
> context: attr;

There is even another similar errors:
/usr/src/xtables-addons-2.12# grep -air "# size_overflow" *
extensions/ACCOUNT/xt_ACCOUNT.o:cicus.321_1094 max, count: 9, decl: # 
size_overflow MARK_NO copy_user_generic 3; num: 0; context: attr;
extensions/ACCOUNT/xt_ACCOUNT.o:cicus.326_1106 max, count: 13, decl: # 
size_overflow MARK_NO copy_user_generic 3; num: 0; context: attr;
extensions/ACCOUNT/xt_ACCOUNT.ko:cicus.321_1094 max, count: 9, decl: # 
size_overflow MARK_NO copy_user_generic 3; num: 0; context: attr;
extensions/ACCOUNT/xt_ACCOUNT.ko:cicus.326_1106 max, count: 13, decl: # 
size_overflow MARK_NO copy_user_generic 3; num: 0; context: attr;
extensions/xt_SYSRQ.o:cicus.199_241 min, count: 10, decl: # 
size_overflow MARK_NO __kmalloc 1; num: 0; context: attr;
extensions/xt_geoip.o:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313 
max, count: 5, decl: # size_overflow MARK_NO copy_user_generic 3; num: 
0; context: attr;
extensions/xt_SYSRQ.ko:cicus.199_241 min, count: 10, decl: # 
size_overflow MARK_NO __kmalloc 1; num: 0; context: attr;
extensions/xt_geoip.ko:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313 
max, count: 5, decl: # size_overflow MARK_NO copy_user_generic 3; num: 
0; context: attr;

And for your information (sorry if it is not readable):
extensions/.xt_geoip.mod.o.cmd:cmd_/usr/src/xtables-addons-2.12/extensions/xt_geoip.mod.o 
:= gcc -Wp,-MD,/usr/src/xtables-addons-2.12/extensions/.xt_geoip.mod.o.d 
  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.8/include 
-I./arch/x86/include -I./arch/x86/include/generated/uapi 
-I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi 
-I./include/uapi -I./include/generated/uapi -include 
./include/linux/kconfig.h -D__KERNEL__ -Wall -Wundef -Wstrict-prototypes 
-Wno-trigraphs -fno-strict-aliasing -fno-common 
-Werror-implicit-function-declaration -Wno-format-security -std=gnu89 
-fno-PIE -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64 
-falign-jumps=1 -falign-loops=1 -mno-80387 -mno-fp-ret-in-387 
-mpreferred-stack-boundary=3 -O3 -march=x86-64 -mno-red-zone 
-mcmodel=kernel -funit-at-a-time -maccumulate-outgoing-args 
-ffreestanding -DCONFIG_X86_X32_ABI -DCONFIG_AS_CFI=1 
-DCONFIG_AS_CFI_SIGNAL_FRAME=1 -DCONFIG_AS_CFI_SECTIONS=1 
-DCONFIG_AS_FXSAVEQ=1 -DCONFIG_AS_SSSE3=1 -DCONFIG_AS_CRC32=1 
-DCONFIG_AS_AVX=1 -DCONFIG_AS_AVX2=1 -DCONFIG_AS_SHA1_NI=1 
-DCONFIG_AS_SHA256_NI=1 -pipe -Wno-sign-compare 
-fno-asynchronous-unwind-tables -fno-delete-null-pointer-checks -O2 
-Wno-maybe-uninitialized --param=allow-store-data-races=0 
-Wframe-larger-than=2048 -fstack-protector -Wno-unused-but-set-variable 
-fomit-frame-pointer -fno-var-tracking-assignments 
-Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow 
-fconserve-stack -Werror=implicit-int -Werror=strict-prototypes 
-DCC_HAVE_ASM_GOTO  
-fplugin=./scripts/gcc-plugins/latent_entropy_plugin.so 
-fplugin=./scripts/gcc-plugins/constify_plugin.so 
-fplugin=./scripts/gcc-plugins/stackleak_plugin.so 
-fplugin=./scripts/gcc-plugins/kernexec_plugin.so 
-fplugin=./scripts/gcc-plugins/colorize_plugin.so 
-fplugin=./scripts/gcc-plugins/size_overflow_plugin/size_overflow_plugin.so 
-fplugin=./scripts/gcc-plugins/randomize_layout_plugin.so 
-fplugin=./scripts/gcc-plugins/structleak_plugin.so 
-fplugin=./scripts/gcc-plugins/initify_plugin.so 
-fplugin=./scripts/gcc-plugins/rap_plugin/rap_plugin.so 
-DLATENT_ENTROPY_PLUGIN -DCONSTIFY_PLUGIN -DSTACKLEAK_PLUGIN 
-fplugin-arg-stackleak_plugin-track-lowest-sp=100 -DKERNEXEC_PLUGIN 
-fplugin-arg-kernexec_plugin-method=bts -DSIZE_OVERFLOW_PLUGIN 
-fplugin-arg-size_overflow_plugin-check-fns 
-fplugin-arg-size_overflow_plugin-check-fields 
-fplugin-arg-size_overflow_plugin-check-fptrs 
-fplugin-arg-size_overflow_plugin-check-vars -DRANDSTRUCT_PLUGIN 
-DSTRUCTLEAK_PLUGIN -DINITIFY_PLUGIN 
-fplugin-arg-initify_plugin-search_init_exit_functions 
-fplugin-arg-initify_plugin-verbose 
-fplugin-arg-initify_plugin-print_missing_attr -DRAP_PLUGIN 
-fplugin-arg-rap_plugin-typecheck=call,ret 
-fplugin-arg-rap_plugin-hash=abs-finish 
-fplugin-arg-rap_plugin-hash=abs-ops 
-fplugin-arg-rap_plugin-hash=abs-attr -DX86_RAP_CALL_VECTOR=0x82 
-DX86_RAP_RET_VECTOR=0x83 '-fplugin-arg-rap_plugin-callabort=int $$0x82' 
'-fplugin-arg-rap_plugin-retabort=int $$0x83'   
-DKBUILD_BASENAME='"xt_geoip.mod"'  -DKBUILD_MODNAME='"xt_geoip"' 
-DMODULE  -c -o /usr/src/xtables-addons-2.12/extensions/xt_geoip.mod.o 
/usr/src/xtables-addons-2.12/extensions/xt_geoip.mod.c


-- 
Best regards,

Loic

Re: [netfilter-core] Heap overflow in xt_geoip.c

[Loic]

>>>> On Tue, Jun 20, 2017 at 08:31:26PM +0200, Loic wrote:
>>>>> Hi,
>>>>> 
>>>>> I think there is a problem in the geoip code because I detect this:
>>>>> 
>>>>> grep -ar "cicus.162_313 max" 
>>>>> /usr/src/xtables-addons-2.12/extensions/
>>>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.o:cicus.162_313 
>>>>> max,
>>>>> count: 7, decl: vmalloc; num: 1; context: fndecl;
>>>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.o:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313
>>>>> max, count: 5, decl: size_overflow MARK_NO copy_user_generic 3; 
>>>>> num:
>>>>> 0; context: attr;
>>>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.ko:cicus.162_313 
>>>>> max,
>>>>> count: 7, decl: vmalloc; num: 1; context: fndecl;
>>>>> /usr/src/xtables-addons-2.12/extensions/xt_geoip.ko:/usr/src/xtables-addons-2.12/extensions/xt_geoip.ccicus.162_313
>>>>> max, count: 5, decl: size_overflow MARK_NO copy_user_generic 3; 
>>>>> num:
>>>>> 0; context: attr;

I did not find what I was looking for but a static code analysis 
revealed a errors.
Help: The documentation for all analyzer warnings is available here: 
http://www.viva64.com/en/w/.

/xtables-addons-2.13/extensions/ACCOUNT/libxt_ACCOUNT_cl.c	166	err	V575 
The null pointer is passed into 'setsockopt' function. Inspect the 
fourth argument.
/xtables-addons-2.13/extensions/ACCOUNT/libxt_ACCOUNT_cl.c	166	err	V575 
The 'setsockopt' function processes '0' elements. Inspect the fifth 
argument.
/xtables-addons-2.13/extensions/pknock/pknlusr.c	45	warn	V641 The size 
of the '& src_addr' buffer is not a multiple of the element size of the 
type 'struct sockaddr'.
/xtables-addons-2.13/extensions/pknock/pknlusr.c	72	warn	V641 The size 
of the '& dest_addr' buffer is not a multiple of the element size of the 
type 'struct sockaddr'.
/xtables-addons-2.13/extensions/xt_DNETMAP.c	401	err	V512 A call of the 
'memcmp' function will lead to the '& e->prefix' buffer becoming out of 
range.
/xtables-addons-2.13/extensions/xt_DELUDE.c	82	warn	V560 A part of 
conditional expression is always true: !oth->rst.
/xtables-addons-2.13/extensions/xt_geoip.c	148	err	V568 It's odd that 
'sizeof()' operator evaluates the size of a pointer to a class, but not 
the size of the '(& geoip_head[proto])->next' class object.
/xtables-addons-2.13/extensions/xt_geoip.c	148	err	V568 It's odd that 
'sizeof()' operator evaluates the size of a pointer to a class, but not 
the size of the 'p->list.next' class object.
/xtables-addons-2.13/extensions/xt_ipp2p.c	514	warn	V666 Consider 
inspecting fourth argument of the function 'HX_memmem'. It is possible 
that the value does not correspond with the length of a string which was 
passed with the third argument.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	622	err	V595 The 
'peer' pointer was utilized before it was verified against nullptr. 
Check lines: 622, 623.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1047	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1053	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1055	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1058	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1061	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1064	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1069	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1072	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1075	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1077	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1079	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1086	warn	V612 An 
unconditional 'return' within a loop.
/xtables-addons-2.13/extensions/pknock/xt_pknock.c	1090	warn	V612 An 
unconditional 'return' within a loop.

Thanks !

-- 
Best regards,

Loic