Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using PCRE JIT maps memory with PROT_WRITE|PROT_EXEC in v3.0.x #1865

Closed
spbnick opened this issue Dec 5, 2016 · 15 comments

Comments

@spbnick
Copy link
Collaborator

commented Dec 5, 2016

Issue type

  • Defect - Crash or memory corruption.
  • Defect - Non compliance with a standards document, or incorrect API usage.
  • Defect - Unexpected behaviour (obvious or verified by project member).
  • Feature request.

Defect/Feature description

Radiusd uses PCRE JIT compiler, which maps memory with PROT_WRITE|PROT_EXEC to
write the resulting code and execute it later. This is picked up and reported
by SELinux:

type=AVC msg=audit(1480946786.958:3174): avc:  denied  { execmem } for  pid=32684 comm="radiusd" scontext=system_u:system_r:radiusd_t:s0 tcontext=system_u:system_r:radiusd_t:s0 tclass=process
type=SYSCALL msg=audit(1480946786.958:3174): arch=c000003e syscall=9 success=no exit=-13 a0=0 a1=10000 a2=7 a3=22 items=0 ppid=1 pid=32684 auid=4294967295 uid=0 gid=95 euid=95 suid=0 fsuid=95 egid=95 sgid=95 fsgid=95 tty=(none) ses=4294967295 comm="radiusd" exe="/usr/sbin/radiusd" subj=system_u:system_r:radiusd_t:s0 key=(null)

This is a problem, because having both writeable and executable memory makes
FreeRADIUS vulnerable to malicious code planting their own executable code
into the mapped area.

The RHEL distribution will likely keep the particular mmap syscall denied
and perhaps silence the error message.

From FreeRADIUS code I see that radiusd can cope with that and that the
optimization simply won't work.

Could you please confirm if this is so, or if you're OK with this behavior?

Thank you.

How to reproduce issue

gdb -ex 'set breakpoint pending on' -ex 'b __mmap if prot == 7' -ex r -ex bt --args radiusd -X

Full backtrace from LLDB or GDB

#0  __mmap (addr=addr@entry=0x0, len=len@entry=65536, prot=prot@entry=7, flags=flags@entry=34, fd=fd@entry=-1, offset=offset@entry=0) at ../sysdeps/unix/sysv/linux/wordsize-64/mmap.c:32
#1  0x00007ffff6cba986 in alloc_chunk (size=65536) at sljit/sljitExecAllocator.c:102
#2  sljit_malloc_exec (size=608) at sljit/sljitExecAllocator.c:213
#3  sljit_generate_code (compiler=compiler@entry=0x555555a768c0) at sljit/sljitNativeX86_common.c:479
#4  0x00007ffff6cd6d4a in _pcre_jit_compile (re=re@entry=0x555555a76630, extra=extra@entry=0x555555a766e0, mode=mode@entry=0) at pcre_jit_compile.c:8207
#5  0x00007ffff6cd9d8d in pcre_study (external_re=0x555555a76630, options=options@entry=1, errorptr=errorptr@entry=0x7fffffffe1a0) at pcre_study.c:1506
#6  0x00007ffff798cf66 in regex_compile (ctx=ctx@entry=0x5555559ea830, out=out@entry=0x7fffffffe210, pattern=pattern@entry=0x5555559ea970 " ", len=1, ignore_case=<optimized out>, multiline=<optimized out>, subcaptures=subcaptures@entry=true, runtime=runtime@entry=false) at src/lib/regex.c:123
#7  0x000055555557e0bf in pass2_regex_compile (vpt=0x5555559ea830, ci=0x5555559ea180) at src/main/modcall.c:3145
#8  pass2_callback (ctx=<optimized out>, c=0x5555559ea450) at src/main/modcall.c:3431
#9  0x00007ffff7bbec4b in fr_condition_walk (c=0x5555559ea450, callback=callback@entry=0x55555557db90 <pass2_callback>, ctx=ctx@entry=0x0) at src/main/parser.c:1758
#10 0x000055555557fe77 in modcall_pass2 (mc=0x555555a69810) at src/main/modcall.c:3611
#11 0x000055555557fc11 in modcall_pass2 (mc=0x555555a69710) at src/main/modcall.c:3907
#12 0x000055555557fc11 in modcall_pass2 (mc=0x555555a69610) at src/main/modcall.c:3907
#13 0x000055555557fc11 in modcall_pass2 (mc=0x555555a69510) at src/main/modcall.c:3907
#14 0x000055555557a94a in virtual_servers_load (config=config@entry=0x5555557e1c00) at src/main/modules.c:1565
#15 0x000055555557b58e in modules_init (config=0x5555557e1c00) at src/main/modules.c:2149
#16 0x00005555555682eb in main (argc=2, argv=0x7fffffffe698) at src/main/radiusd.c:472
@arr2036

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

FreeRADIUS will error out on startup if compilation fails. If pcre_study doesn't return an error code it should be fine.

@spbnick

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 5, 2016

I think pcre_study will return an error, although I wasn't able to test it quickly (SELinux wouldn't deny it to me and is too messy to figure out). However, radiusd doesn't seem to care and doesn't check the returned value, which would be NULL in case of failure, which actually works fine with pcre_exec later.

@arr2036

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

It might be an idea to submit a patch to libpcre to call mprotect with PROT_READ | PROT_EXEC after JIT compilation has completed. That should remove the potential attack vector... at least for FreeRADIUS.

@spbnick

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 5, 2016

@arr2036 Yes, I'll leave that as an option. Although SELinux would still be offended.

@alandekok

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

The simplest thing to do is to add PCRE2_NO_JIT to the pcre options.

Since the pcre code is in src/lib/regex.c, it can't really be a configuration option in the server. I'm happy to accept a patch which just uses it on RedHat. i.e.

#ifdef WITH_PCRE2_NO_JIT
   flags |= PCRE2_NO_JIT
#endif  

Or something like that. If RH is disabling it globally, there's no reason to have it a config option.

@spbnick

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 5, 2016

Thank you for your answer, @alandekok, but sorry, I'm not familiar with PCRE2_NO_JIT and can't find it anywhere. Could you please elaborate?

@alandekok

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

There should be a run-time flag for PCRE which says "don't use the jit".

$ grep JIT /usr/local/include/pcre*.h

On the recent versions of PCRE, there's a PCRE2_NO_JIT flag. On older ones, it isn't there.

Ultimately, we're limited by the underlying libraries. If they don't support the security that RH needs, then no amount of poking FreeRADIUS will fix it. The only solution is to build PCRE on RedHat without any JIT support. And that's something we can't help with.

@spbnick

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 5, 2016

Sure, I understand it's PCRE doing it. There was a similar problem with grep regexes being used by CUPS (https://bugzilla.redhat.com/show_bug.cgi?id=1079534) in 2014.

However, FreeRADIUS may opt to simply not supply PCRE_STUDY_JIT_COMPILE to pcre_study. Then there would be no JIT compilation, IIUC. Or perhaps there could be a runtime option to turn it on for those really needing it and willing to compromise safety a little.

Do you think you could go that way?

@alandekok

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

It would help, but it wouldn't solve the issue. FreeRADIUS already passes PCRE_STUDY_JIT_COMPILE for fixed regexes.

The issue (I think) is that FreeRADIUS allows for run-time creation of regexes. Which is the main problem.

The only solution to that is to fix PCRE.

@spbnick

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 5, 2016

Ah, finally found PCRE2_NO_JIT. I haven't realized they had a separate repo for pcre2.

It would help, but it wouldn't solve the issue. FreeRADIUS already passes PCRE_STUDY_JIT_COMPILE for fixed regexes.

Do you mean that you don't want to change the default to not have JIT? Or there's some other reason not to make PCRE_STUDY_JIT_COMPILE optional? Sorry, I'm kind of slow today.

By fixing PCRE do you mean adding remapping without PROT_WRITE after compilation? If so I'm not sure they'd like it much, and it would still trigger SELinux. Although, yes, it would make it somewhat safer. I'll keep that option on the table.

@spbnick

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 5, 2016

I mean I just tried passing zero instead of PCRE_STUDY_JIT_COMPILE and SELinux had no problem with that.

@alandekok

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

That looks like it's an OS-specific solution, then. I'm Ok with patches to disable that on RH, but the option still has to be functional on other systems. The speed improvement with the JIT can be significant.

@arr2036

This comment has been minimized.

Copy link
Member

commented Dec 5, 2016

Just looked at this some more, and fixed up the JIT code in v4.0.x to make it more forgiving... But i'm not sure if that's required.

In theory you should just be able to configure libpcre with --disable-jit and it'll just ignore the PCRE_STUDY_JIT_COMPILE flag when passed to pcre_study.

Otherwise every consumer of libpcre on an RHEL system could have the same issues with mmap and SELinux?

@arr2036

This comment has been minimized.

Copy link
Member

commented Dec 6, 2016

PCRE2_NO_JIT is for a different version of the library than we use. pcre2 was released in 2015 and has a different API.

@arr2036

This comment has been minimized.

Copy link
Member

commented Dec 9, 2016

Closing this for now... It's really not an issue with the FreeRADIUS code. If this feature of libpcre is seen as a security vulnerability on RHEL systems, then it should just be disabled in libpcre...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.