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

Control Flow Integrity #3810

Open
defuse opened this issue Jan 23, 2019 · 6 comments
Open

Control Flow Integrity #3810

defuse opened this issue Jan 23, 2019 · 6 comments
Labels
F-clang Feature: Clang support I-SECURITY Problems and improvements related to security. special to Taylor

Comments

@defuse
Copy link
Contributor

defuse commented Jan 23, 2019

Control Flow Integrity (CFI) is an approach to mitigating code-reuse attacks (e.g. ROP). In this ticket, evaluate the CFI options that are available and see if any would be beneficial to zcashd. A quick search returns:

@defuse defuse added I-SECURITY Problems and improvements related to security. special to Taylor labels Jan 23, 2019
@daira
Copy link
Contributor

daira commented Jan 28, 2019

Microsoft CFG seems inapplicable unless the code is compiled with MSVC, which we don't support, and I think we're unlikely to support.

We do support compiling with clang, at least for Thread/Address Sanitizer builds. (I don't know whether those still require a patch, but if so it wouldn't be difficult to fix.)

@daira daira added the F-clang Feature: Clang support label Jan 28, 2019
@daira daira added this to Needs Prioritization in Arborist Team via automation Jan 28, 2019
@daira
Copy link
Contributor

daira commented Jan 28, 2019

Note that according to https://llvm.org/docs/GoldPlugin.html you need to enable the Gold linker when compiling, using -fuse-ld=gold. (We can't require the user to change their system linker.)

If I understand correctly, as long as we still dynamically link to the system libc, this won't prevent return-to-libc attacks, and similarly for libstdc++ and other dynamically linked system libraries (see #2513).

@daira
Copy link
Contributor

daira commented Jan 28, 2019

It looks like we have to solve #2513 first in order for the cfi-mfcall check to work:

For this scheme to work, all translation units containing the definition of a virtual member function (whether inline or not), other than members of blacklisted types or types with public LTO visibility, must be compiled with -flto or -flto=thin enabled and be statically linked into the program.

because libstdc++ will contain such virtual member functions.

(Note that #2513 is currently blocked by #3744.)

@mms710 mms710 moved this from Needs Prioritization to Sprint Backlog in Arborist Team Jan 28, 2019
@zebambam
Copy link

We can still enable the other checks though, right?

@zebambam
Copy link

If I understand correctly, as long as we still dynamically link to the system libc, this won't prevent return-to-libc attacks, and similarly for libstdc++ and other dynamically linked system libraries (see #2513).

Right, but ASLR puts standard libraries at random locations, making ret-to-libc harder.

In general the incompleteness of one mitigation technology shouldn't remove it from our consideration. All of them are incomplete, we should consider them and their effect as a set, and one that we improve over time.

@mms710 mms710 moved this from Sprint Backlog to Blocked in Arborist Team Jan 29, 2019
@mms710 mms710 moved this from Blocked to Sprint Backlog in Arborist Team Jan 29, 2019
@zebambam
Copy link

I think you just need LTO, which I think has been available for a while. It looks like the gold recommendation was just a suggestion. In general though, I think users who build zcashd without a given security option because it's unavailable for some reason on their platform should have to explicitly disable that, and the build should fail if they don't disable something that isn't available on their platform.

@mms710 mms710 moved this from Bugs/TechDebt Backlog to Security Issue Backlog in Arborist Team May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-clang Feature: Clang support I-SECURITY Problems and improvements related to security. special to Taylor
Projects
Arborist Team
  
Security Issue Backlog
Development

No branches or pull requests

3 participants