Opened 16 months ago

Last modified 4 months ago

#22660 needs_revision defect

Guard against stack smashing attacks in tor with additional compiler options.

Reported by: teor Owned by: nickm
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hardening, security, 029-backport, 032-backport, 033-backport, review-group-19, 032-unreached, 034-triage-20180328, 034-removed-20180328, 031-unreached-backport
Cc: Actual Points:
Parent ID: Points: 0.5
Reviewer: Sponsor:

Description (last modified by teor)

If we tor with -fstack-check (GCC, it's a no-op in clang[0]), it will protect against stack smashing attacks that jump the stack guard page(s):

Recompile all userland code (ld.so, libraries, binaries) with GCC's
  "-fstack-check" option, which prevents the stack-pointer from moving
  into another memory region without accessing the stack guard-page (it
  writes one word to every 4KB page allocated on the stack).

III Solutions, https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt

"
-fstack-check
Generate code to verify that you do not go beyond the boundary of the stack. You should specify this flag if you are running in an environment with multiple threads, but only rarely need to specify it in a single-threaded environment since stack overflow is automatically detected on nearly all systems if there is only one stack.
Note that this switch does not actually cause checking to be done; the operating system must do that. The switch causes generation of code to ensure that the operating system sees the stack being extended.
"
http://gcc.gnu.org/onlinedocs/gcc-4.3.6/gcc/Code-Gen-Options.html#Code-Gen-Options

This protects against:

- a local-root exploit against ld.so and most SUID-root binaries
  (CVE-2017-1000366, CVE-2017-1000379) on amd64 Debian, Ubuntu, Fedora,
  CentOS;

There are remote attack possibilities mentioned in the paper as well.

We might also want to add:

-Wl,-z,noexecstack and -Wl,-z,noexecheap

https://www.owasp.org/index.php/C-Based_Toolchain_Hardening#GCC.2FBinutils

[0]: for clsng, we could use -fsanitize=safe-stack instead, but that's more intrusive: https://blog.quarkslab.com/clang-hardening-cheat-sheet.html

Child Tickets

Change History (17)

comment:1 Changed 16 months ago by teor

Description: modified (diff)

My internal wiki wysiwyg is broken.

comment:2 Changed 16 months ago by nickm

Keywords: 029-backport 030-backport 031-backport added
Owner: set to nickm
Status: newaccepted

We already have -fstack-protector-all, but I guess it doesn't include this?

Also, we should never be installed SUID, yeah?

That said, this is indeed a pretty trivial change; see my branch harden_22660_029, for 0.3.2 and possible backport as far as 0.2.9

When I try to build with this, I get many /usr/bin/ld: warning: -z noexecheap ignored. warnings. I don't know if that's something I should be concerned about.

comment:3 Changed 16 months ago by nickm

Status: acceptedneeds_review

comment:4 in reply to:  2 Changed 16 months ago by teor

Replying to nickm:

We already have -fstack-protector-all, but I guess it doesn't include this?

Also, we should never be installed SUID, yeah?

tor relays often launch tor as root, and read their configs, then downgrade, but I don't know of any common setups that allow a local unprivileged user to influence its torrc. (If there were, that user could gain access to at least the tor user.)

I'd be more worried about network attacks based on research like this: the researchers couldn't rule them out.

Edit: launch as root then downgrade

When I try to build with this, I get many /usr/bin/ld: warning: -z noexecheap ignored. warnings. I don't know if that's something I should be concerned about.

I wonder if your linker supports noexecheap?

Last edited 16 months ago by teor (previous) (diff)

comment:5 Changed 16 months ago by cypherpunks

You don't need -z options.

I wonder if your linker supports noexecheap?

"Other keywords are ignored for Solaris compatibility." from https://sourceware.org/binutils/docs-2.28/ld/Options.html#Options

comment:6 Changed 16 months ago by nickm

Keywords: review-group-19 added

comment:7 Changed 16 months ago by cypherpunks

https://twitter.com/CopperheadOS/status/876835207701200896
seclists.org/oss-sec/2017/q2/505
openwall.com/lists/oss-security/2017/06/19/3

comment:8 Changed 16 months ago by dgoulet

Status: needs_reviewneeds_revision

I do also get /usr/bin/ld: warning: -z noexecheap ignored. quite a bit.

I think it's because it simply doesn't exist on my system. I can't find that noexecheap in the man page ld.1. According to https://www.owasp.org/index.php/C-Based_Toolchain_Hardening#GCC.2FBinutils, it was added in binutils 2.14 but I'm on 2.28... I've looked through the source code of binutils package and I can't find that noexecheap string so it might have been removed?...

Maybe TOR_CHECK_LDFLAGS() doesn't fully detect the "ignored parameter" because it's not an "error"? dunno

comment:9 Changed 16 months ago by teor

llvm has just added stack probing support to master, so hopefully clang will make -fstack-check work eventually:
http://llvmweekly.org/issue/182
http://reviews.llvm.org/rL305939

Rust is implementing stack probing using the new llvm stack probing support:
https://github.com/rust-lang/rust/pull/42816

comment:10 Changed 13 months ago by nickm

Summary: Guard against stack smashing attacks in torGuard against stack smashing attacks in tor with additional compiler options.

comment:11 Changed 13 months ago by nickm

Keywords: 032-unreached added
Milestone: Tor: 0.3.2.x-finalTor: unspecified

Mark a large number of tickets that I do not think we will do for 0.3.2.

comment:12 Changed 9 months ago by nickm

Keywords: 030-backport removed

Remove 030-backport from all open tickets that have it: 0.3.0 is now deprecated.

comment:13 Changed 8 months ago by cypherpunks

Keywords: 032-backport 033-backport added
Milestone: Tor: unspecifiedTor: 0.3.4.x-final

Hardening is useful.

comment:14 Changed 7 months ago by nickm

Keywords: 034-triage-20180328 added

comment:15 Changed 7 months ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:16 Changed 7 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These needs_revision, tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if somebody does the necessary revision.

comment:17 Changed 4 months ago by teor

Keywords: 031-unreached-backport added; 031-backport removed

0.3.1 is end of life, there are no more backports.
Tagging with 031-unreached-backport instead.

Note: See TracTickets for help on using tickets.