Opened 5 months ago

Last modified 3 months ago

#31183 new defect

Situational symlink attacks on ControlPortWriteToFile etc.

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: hackerone, bug-bounty, security, 041-backport?, 042-deferred-20190918
Cc: Actual Points:
Parent ID: Points: 1
Reviewer: Sponsor:

Description

Here is a bug report from paldium on hackerone. It's basically a very situational and restricted local priviledge escalation against certain setups and threat models:

# Summary

It is possible to change permissions of files through tor or
gain write access to newly created ones. The target file can be
chosen by a local attacker if an adjusted configuration is used.

#How to reproduce

- Given is a FreeBSD or Mac OS X system.
- Tor is configured with a torrc file containing this line:
    ControlPortWriteToFile /tmp/control.txt
- Optionally (race condition) this line is included as well:
    ControlPortFileGroupReadable 1
- /tmp is a directory with sticky bit, i.e. trwxrwxrwx

##or

- Given is a Unix-like (Linux) system.
- Tor is configured with a torrc file containing this line:
    ControlPortWriteToFile /tmp/proof/control.txt
- Optionally (race condition) this line is included as well:
    ControlPortFileGroupReadable 1
- /tmp/proof is writable by the local attacker

When tor starts, then it will eventually write all available control
ports into file configured by "ControlPortWriteToFile". This file is not
written directly into, but a temporary file is used (the extension
".tmp" is added to file name). If this file already exists, then it is
simply truncated.

See src/lib/fs/files.c start_writing_to_file for details, especially
line 321 and following.

The problem is that an attacker can simply create the temporary file
before tor gets the chance to. On Mac this attack works by default
against /tmp, but Linux has a protection against symlink attacks on
directories with sticky bit like /tmp or /var/tmp. Therefore it takes an
unusual configuration on Linux or a different (regular) directory.

Let the attacker create a file which tor, even with dropped privileges,
can write to:

`attacker$ install -m 666 /dev/null /tmp/control.txt.tmp`

The tor process will use this file without adjusting the permissions,
because O_CREAT was not actually performed. Instead, O_TRUNC simply
truncated the file to remove existing content.

Afterwards tor will write content to this file, which the attacker could
simply override at this point. But there is no real need to, because
the target file will still keep the permissions of this temporary file
after rename.

See src/lib/fs/files.c replace_file for details. Basically it's a
simple rename() call which therefore removes the target inode and
renames the temporary file to the target one (in this setup).


At this point, the first attack is finished. The file /tmp/control.txt
is under control of the attacker. If "ControlPortFileGroupReadable 1"
is not given, this exploit code works on Linux systems as well (you
can skip the proof/ part on Mac due to lack of /tmp protection):

```In torrc: ControlPortWriteToFile /tmp/proof/control.txt
attacker$ install -Dm 666 /dev/null /tmp/proof/control.txt.tmp
root$ tor -f torrc
attacker$ ls -l /tmp/proof/control.txt
-rw-rw-rw- 1 attacker  attacker    0 Jun 24 22:59 /tmp/proof/control.txt```


##Second attack

The second attack uses a race condition. Because /tmp/control.txt is
under control of the attacker, the file can be deleted and replaced with
a symbolic link to a target which we want to get group read permissions
for. This is a possible scenario if "ControlPortFileGroupReadable 1" is
given.

In this case, chmod() is called in tor process. See
src/feature/control/control.c control_ports_write_to_file for more
details, especially line 149 for more details (chmod).

The system call chmod() uses a file path as an argument. There is no
guarantee that the file path still refers to the file which has been
created by the tor process. Furthermore, chmod() follows symbolic
links, therefore the referenced target file is adjusted by chmod.

This is obviously a race condition but can be used to gain read access
to files which -- even by configuration -- should be private to the
tor user.

The attacker still needs group-read permissions, otherwise the newly
revealed files still cannot be accessed.

##Example

`tor just renamed controlled /tmp/control.txt.tmp to /tmp/control.txt`
`attacker$ rm /tmp/control.txt`
`attacker$ ln -s /var/lib/tor /tmp/control.txt`
`tor calls chmod on /tmp/control.txt and therefore on /var/lib/tor`
The attacker, if part of tor's group, can access /var/lib/tor now as well
(only read, but hierarchy is known through other tor installations)

#Solution

Use fchmod() on a file which has been opened with open() and O_NOFOLLOW
to prevent changing any files which have been reached through symbolic
links. Preventing symbolic links is already a great deal in file
src/lib/fs/dir.c check_private_dir, line 85. Could be implemented as
tor_chmod() to fix other chmod() calles in the code as well.

Use mkstemp() to atomically create a temporary file which has the
guaranteed permission 600 and owned by the tor user.

## Impact

A local attacker can modify the content of files which are considered trusted by dependent tools, e.g. the control port file.
Also a local attacker can extend privileges of files which are supposed to be private to the tor user and not readable by the group.

Timeline:
2019-06-25 10:24:59 +0000: @paldium (comment)
To clarify one aspesct about attack 1: Of course it is not a tor issue if someone creates a world writable directory and lets tor create files in there. Everyone could simply override the resulting file and tor will never be able to prevent that. Therefore, the rename()-issue is a non-issue on Linux. But as a user I would expect that temporary files within /tmp with sticky bits are properly handled.

But attack 2 (race condition) should even be prevented in a world writable directory (or writable by a possibly malicious user). Therefore I consider attack 2 on Linux to be unlikely but plausible.

I would like to add a patch to this, but I am not sure how you want to handle file permissions.

The nicest approach would be to supply the permissions to the function which creates the file, but that would be a no-op on Windows.

Otherwise tor_chmod could open() the file and at least prevent symbolic link attacks. Yet, files could still be modified which are not the ones we created.

As this is a design decision, I haven't started writing a patch because I do not know which one you would prefer.

Here is some further information:

ControlPortWriteFile is one example. The same attack scenarios are true for:
- ExtORPortCookieAuthFileGroupReadable 1
- CookieAuthFileGroupReadable 1
- DataDirectoryGroupReadable 1
- CacheDirectoryGroupReadable 1
- KeyDirectoryGroupReadable 1

Here is some of my analysis for attack scenarios:

Hello @paldium,

thanks for submitting this report.

Here is the best attacks we could find given the bugs you gave us:

Attack 1) The most realistic attack scenario I could think of is a system where the attacker is a local user who cannot establish outgoing connections, but is able to overwrite the ControlPortWriteToFile file, and replaces it with an attacker controlled IP:PORT, and then a controller program connects to the evil IP:PORT thereby deanonymizing the user. This seems to be a very artificial scenario, which assumes a particular threat model, and a tor with specific configuration parameters, and a very specific system,.

Attack 2) The most realistic attack scenario here would be the attacker using this "read anything controlled by Tor" race condition attack to learn the private keys of an onion service that are on the same system but not able to read them... I'm actually not sure if that would work, but I think it's possible. This also assumes a particular threat-model, a multi-user system, and specific configuration parameters.

@paldium, would it be possible to outline the various solutions we have for fixing this issue?I don't think specifying the permissions in the torrc is a nice thing from a UX perspective. Perhaps not following symlinks is a start but not the whole thing? Maybe we should just abort if there is a dangerous configuration? I wonder how prevalent this is.

and here is suggestions on patching:

Hi @asn,

I agree here, the attack is impossible against default setups and takes quite specific steps to be exploitable.

To fix this vulnerability at its root, I recommend to adjust the function `start_writing_to_file` in `src/lib/fs/files.c`. The system call `mkstemp` (included in POSIX) guarantees to create a unique file name for the (optional) temporary file. This way, an attacker cannot prepare a file before tor tries to create it. In case of conflict, mkstemp iterates through a huge pool of possible names and if all fail, it returns -1.

It must be checked if mkstemp is a viable option on all target systems, especially Windows. But it's POSIX, so it should work.

The attached patch performs these changes, but breaks a test which would be redundant then (it fails because it tries to create a temporary file beforehand, which `mkstemp` successfully prevents).

###Next improvement to consider (for attack 2):

As far as I understand the code, possible modes for files are:
- 0600 (default)
- 0640 (if configuration requests group-writable files, non-Windows systems only)
- 0400 (tor-gencert, which is not expected to run on Windows according to manual page)

If the special case 0400 is handled in tor-gencert directly, the functions in `src/lib/fs/files.c` can be further reduced in their feature set: Just add a "group readable" attribute to these functions and remove the explicit `mode` (if present):
- start_writing_to_stdio_file
- start_writing_to_file
- write_str_to_file
- write_bytes_to_file

All these functions would use 0600 by default and only support 0640 if the boolean "group readable" flag is set to true -- and that will only happen on non-Windows systems.

With these changes in place, the remaining `chmod` calls (except for the control socket) can be removed and that will also fix the second attack (and gives full atomic control to the newly introduced `fchmod` call):

###Before:

```
  if (write_str_to_file(options->ControlPortWriteToFile, joined, 0) < 0) {
    log_warn(LD_CONTROL, "Writing %s failed: %s",
             options->ControlPortWriteToFile, strerror(errno));
  }
#ifndef _WIN32
  if (options->ControlPortFileGroupReadable) {
    if (chmod(options->ControlPortWriteToFile, 0640)) {
      log_warn(LD_FS,"Unable to make %s group-readable.",
               options->ControlPortWriteToFile);
    }
  }
#endif /* !defined(_WIN32) */
```
###After:
```
  if (write_str_to_file(options->ControlPortWriteToFile,
                        options->ControlPortFileGroupReadable, joined, 0) < 0) {
    log_warn(LD_CONTROL, "Writing %s failed: %s",
             options->ControlPortWriteToFile, strerror(errno));
  }

Child Tickets

Change History (2)

comment:1 Changed 5 months ago by nickm

Keywords: 041-backport? added; 041-should? removed

comment:2 Changed 3 months ago by nickm

Keywords: 042-deferred-20190918 added
Milestone: Tor: 0.4.2.x-finalTor: unspecified

Deferring various tickets from 0.4.2 to Unspecified.

Note: See TracTickets for help on using tickets.