Opened 5 years ago

Closed 4 years ago

#12468 closed defect (fixed)

TBB unconditionally logs all Firefox output to disk

Reported by: rransom Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: TorBrowserTeam201503R, MikePerry201503R, tbb-4.5-alpha
Cc: mikeperry, isis, gk, nickm, arma, phobos Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Previously, Tor Browser Bundle's startup script would check whether its stdout and stderr are pointed at a terminal, and close them otherwise (unless the user is explicitly running TBB in debug mode). This allowed power users to easily view TBB's error messages in a terminal, while ensuring that novice users who start TBB from a GUI file manager would not accidentally leave evidence that they had used TBB in ~/.xsession-errors.

Now, the startup script unconditionally redirects Firefox's output into a disk file, and according to 11751#comment:6, that disk file will contain sensitive information (“it often includes full URIs logged from HTTPSEverywhere”).

Child Tickets

Attachments (6)

detect_issue.sh (3.4 KB) - added by cypherpunks 5 years ago.
startup script wrongly report as running in terminal for some platforms
list-ttys.sh (267 bytes) - added by rransom 5 years ago.
list which standard FDs are terminals
0001-Handle-debug-earlier-in-RelativeLink.sh.patch (1.7 KB) - added by rransom 5 years ago.
patches to avoid Unity logging
0002-Send-complain-messages-to-stderr-only-in-debug-mode.patch (2.6 KB) - added by rransom 5 years ago.
patches to avoid Unity logging
0003-Close-stdout-err-immediately-after-debug-mode-check-.patch (2.1 KB) - added by rransom 5 years ago.
patches to avoid Unity logging
0004-Close-stdout-and-stderr-whenever-not-entering-debug-.patch (2.0 KB) - added by rransom 5 years ago.
patches to avoid Unity logging

Download all attachments as: .zip

Change History (23)

comment:1 Changed 5 years ago by mikeperry

The problem is that this logging to disk happens on Mac and likely also Windows already. If we also save a linux log file, we can more easily chase down disk leaks like #12472 and others that apply to those users.

However, since we can and probably should fix #12472 first, I reverted c2bd8a5f070e771623aebfb66e95e9e954e5522d until we have an HTTPS-Everywhere alpha with that fix.

I'm going to leave this open though because Tor Browser still unconditionally logs to disk on Mac and probably Windows. Patches welcome!

comment:2 Changed 5 years ago by cypherpunks

Some platforms wrongly associates stdout and stderr with a terminal device. Tor Browser Bundle's startup script broken for them if no terminal present, stdout leaks out and depends system written to some logs. Like unity+nautilus. Attaching script that detects issue.

Changed 5 years ago by cypherpunks

Attachment: detect_issue.sh added

startup script wrongly report as running in terminal for some platforms

Changed 5 years ago by rransom

Attachment: list-ttys.sh added

list which standard FDs are terminals

comment:3 in reply to:  2 Changed 5 years ago by rransom

Replying to cypherpunks:

Some platforms wrongly associates stdout and stderr with a terminal device. Tor Browser Bundle's startup script broken for them if no terminal present, stdout leaks out and depends system written to some logs. Like unity+nautilus.

Which version of Ubuntu are you using? (I can't easily test on Ubuntu now, but other people will want to know.) Are you using the standard configuration/state (i.e. default configuration of Unity started from the default desktop manager, and without running Nautilus from a terminal)?

Which file descriptors are incorrectly reported as terminals?

Are messages that a program sends to stdout displayed to the user immediately? Are messages that a program sends to stderr displayed to the user immediately?

Are messages that a program sends to stdout logged to disk, and if so, where? Are messages that a program sends to stderr logged to disk, and if so, where?

Attaching script that detects issue.

if [ "$ARE_WE_RUNNING_IN_A_TERMINAL" -ne 0 ] && [ '!' -t 0 ]; then
    complain "BUG: Wrongly reported as running in terminal"
else
   echo "Correct. Running in terminal" >&1
fi

In the real script, ARE_WE_RUNNING_IN_A_TERMINAL only controls whether complain will write its message to stderr or run an X program to display the message. The security checks are (currently) on lines 139 through 146, and close whichever of stdout or stderr is not a terminal.

It's actually perfectly reasonable for ARE_WE_RUNNING_IN_A_TERMINAL to equal 1 when stdin is not a terminal: for example, if a user types ./start-tor-browser </dev/null in a terminal. ARE_WE_RUNNING_IN_A_TERMINAL can also be 1 if e.g. stdout is redirected to a file, in which case complain will still write to stderr. (This is why the security checks had to be separate from that variable.)

I've attached a script (list-ttys.sh) to diagnose the state of a script's standard FDs more precisely.

It is bad to have ARE_WE_RUNNING_IN_A_TERMINAL set to 1 when the user won't see messages sent to stderr, but that's a user-interface problem, and not just with TBB. It would be much worse to have stdout attached to a pty that gets logged to disk, and I can't imagine that anyone would do that.

comment:4 Changed 5 years ago by cypherpunks

Which version of Ubuntu are you using? (I can't easily test on Ubuntu now, but other people will want to know.) Are you using the standard configuration/state (i.e. default configuration of Unity started from the default desktop manager, and without running Nautilus from a terminal)?

It happens for Ubuntu 14.04. It's possible to test with installation CD in virtual environment. No any changes need.

Which file descriptors are incorrectly reported as terminals?

1 and 2. (stdout and stderr)

Are messages that a program sends to stdout displayed to the user immediately? Are messages that a program sends to stderr displayed to the user immediately?

Ubuntu writes such stdout to .cache/upstart/gnome-session-Unity.log. Invisible for user.

comment:5 Changed 5 years ago by cypherpunks

list-ttys.sh is running...
stdout is a tty
stderr is a tty
list-ttys.sh is ending

comment:6 Changed 5 years ago by cypherpunks

No any changes need

Except changes to nautilus preferences.

comment:7 Changed 5 years ago by cypherpunks

If Ubuntu (or whatever internalies) violates basis then script should to detect broken platform and terminate execution.

comment:8 Changed 5 years ago by cypherpunks

I fixed logging of stdout. Deleted Ubuntu and no anymore problem. Ubuntu is not ready for security.

comment:9 Changed 5 years ago by cypherpunks

deleted. not working with ./start-tor-browser </dev/null

Last edited 5 years ago by cypherpunks (previous) (diff)

Changed 5 years ago by rransom

patches to avoid Unity logging

Changed 5 years ago by rransom

patches to avoid Unity logging

Changed 5 years ago by rransom

patches to avoid Unity logging

Changed 5 years ago by rransom

patches to avoid Unity logging

comment:10 in reply to:  4 Changed 5 years ago by rransom

Replying to cypherpunks:

Which version of Ubuntu are you using? (I can't easily test on Ubuntu now, but other people will want to know.) Are you using the standard configuration/state (i.e. default configuration of Unity started from the default desktop manager, and without running Nautilus from a terminal)?

It happens for Ubuntu 14.04. It's possible to test with installation CD in virtual environment. No any changes need.

I can't currently run a VM.

Which file descriptors are incorrectly reported as terminals?

1 and 2. (stdout and stderr)

Are messages that a program sends to stdout displayed to the user immediately? Are messages that a program sends to stderr displayed to the user immediately?

Ubuntu writes such stdout to .cache/upstart/gnome-session-Unity.log. Invisible for user.

Crud. Using a pty for that is a horrible idea, and it'll break more than just TBB.

I've attached a series of (untested) patches that should turn off all output to stdout and stderr unless the user explicitly passes the --debug option.

comment:11 Changed 5 years ago by erinn

Keywords: needs-triage added

comment:12 Changed 5 years ago by erinn

Component: Tor bundles/installationTor Browser
Keywords: needs-triage removed
Owner: changed from erinn to tbb-team

comment:13 in reply to:  1 Changed 5 years ago by isis

Replying to mikeperry:

The problem is that this logging to disk happens on Mac and likely also Windows already. If we also save a linux log file, we can more easily chase down disk leaks like #12472 and others that apply to those users.

However, since we can and probably should fix #12472 first, I reverted c2bd8a5f070e771623aebfb66e95e9e954e5522d until we have an HTTPS-Everywhere alpha with that fix.

I'm going to leave this open though because Tor Browser still unconditionally logs to disk on Mac and probably Windows. Patches welcome!


It is now believed that my c2bd8a5f070e771623aebfb66e95e9e954e5522d commit (not the logging to file part, but the rest of it) might fix #12941 for Linux users. We probably want it, and if necessary I can revise it to remove the logfile portion and keep the rest.

Also, while looking over HTTPSEverywhere a couple weeks ago, with a fresh copy of Tor Browser, HTTPSEverywhere appeared to no longer logging URIs. (My friend from the EFF and I used it for about 20 minutes or so and didn't see any.) Perhaps if HTTPSEverywhere has fixed this, then logging to file is no longer such a drastic issue?

comment:14 Changed 5 years ago by gk

#13542 is probably related to this.

Last edited 5 years ago by gk (previous) (diff)

comment:15 in reply to:  14 Changed 5 years ago by gk

Replying to gk:

#13542 is probably related to this.

Ah, no, scratch that.

comment:16 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201503R MikePerry201503R added
Priority: blockernormal

Hrmm, my policy of ignoring rage-blocker tickets caused me to miss the patches here. Sorry about that. Patch 0002 seems like it removes a good pile of cruft that is worth trimming from start-tor-browser.

comment:17 Changed 4 years ago by mikeperry

Keywords: tbb-4.5-alpha added
Resolution: fixed
Status: newclosed

Robert's patches were merged for 4.5a5 as part of #13375. I had to modify them to deal with bitrot and other #13375 changes, so I set myself as the author and credited Robert in the commit description. I hope that is OK. (I was working off the assumption that Robert is the type of person who would prefer that modified patches based on his work should not be attributed to him directly.)

https://gitweb.torproject.org/builders/tor-browser-bundle.git/commit/?id=b734892e76d0fd8d45fbd56da1cede14df8f86a9

Note: See TracTickets for help on using tickets.