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

tinyproxy should create its PID file before dropping privileges (CVE-2017-11747) #106

Closed
orlitzky opened this Issue Jul 28, 2017 · 29 comments

Comments

Projects
None yet
4 participants
@orlitzky

orlitzky commented Jul 28, 2017

Summary

The tinyproxy daemon should create its PID file before dropping
privileges. This represents a minor security issue; additional factors
are needed to make it exploitable.

Description

The purpose of the PID file is to hold the PID of the running daemon,
so that later it can be stopped, restarted, or otherwise signalled
(many daemons reload their configurations in response to a SIGHUP).
To fulfill that purpose, the contents of the PID file need to be
trustworthy. If the PID file is writable by a non-root user, then he
can replace its contents with the PID of a root process. Afterwards,
any attempt to signal the PID contained in the PID file will instead
signal a root process chosen by the non-root user (a vulnerability).

This is commonly exploitable by init scripts that are run as root and
which blindly trust the contents of their PID files. If one daemon
flushes its cache in response to SIGUSR2 and another daemon drops all
connections in response to SIGUSR2, it is not hard to imagine a
denial-of-service by the user of the first daemon against the second.

Exploitation

There is only a risk of exploitation when some other user relies on
the data in the PID file. But you have to wonder, what's the point of
the PID file if not to provide the PID to other people? Any situation
where the PID file is used is therefore suspicious.

@tb3088

This comment has been minimized.

Show comment
Hide comment
@tb3088

tb3088 Jul 28, 2017

well, here-in lies a debate: Tiny proxy should never run as root even temporarily unless it's binding to a low port, period. My initscript is perfectly valid to be of the form: 'su user tinyproxy -c blah'.

there are many, many programs that own and write arbitrary values to their pid files. I don't think we need to make tiny insulate itelf from stupid sysadmins. There are no wide-spread reports that I know of people trashing their systems by carelessly signalling the arbitrary value.

It is current-day practice for /var/run to be mode 1777 even. I do welcome you brininging it up, since /etc/init.d/functions:killproc() doesn't check for '1'. ala
# kill it
if [ -n "$pid" ] ; then

tb3088 commented Jul 28, 2017

well, here-in lies a debate: Tiny proxy should never run as root even temporarily unless it's binding to a low port, period. My initscript is perfectly valid to be of the form: 'su user tinyproxy -c blah'.

there are many, many programs that own and write arbitrary values to their pid files. I don't think we need to make tiny insulate itelf from stupid sysadmins. There are no wide-spread reports that I know of people trashing their systems by carelessly signalling the arbitrary value.

It is current-day practice for /var/run to be mode 1777 even. I do welcome you brininging it up, since /etc/init.d/functions:killproc() doesn't check for '1'. ala
# kill it
if [ -n "$pid" ] ; then

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Jul 28, 2017

@tb3088 and how does your init script stop tinyproxy?

orlitzky commented Jul 28, 2017

@tb3088 and how does your init script stop tinyproxy?

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Jul 28, 2017

I would also like to know what distributions have /var/run or /run marked o+rwx.

orlitzky commented Jul 28, 2017

I would also like to know what distributions have /var/run or /run marked o+rwx.

@tb3088

This comment has been minimized.

Show comment
Hide comment
@tb3088

tb3088 Jul 28, 2017

the LSB init scripts have for years not required a pid file. They use things like pidof or pgrep.
I believe all the RHEL derivatives do. Not sure where I got that idea... Ubuntu 16.04 has /run root:root 755. I checked a recent Amazon Linux. Same thing.

tb3088 commented Jul 28, 2017

the LSB init scripts have for years not required a pid file. They use things like pidof or pgrep.
I believe all the RHEL derivatives do. Not sure where I got that idea... Ubuntu 16.04 has /run root:root 755. I checked a recent Amazon Linux. Same thing.

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Jul 28, 2017

755 and 777 are not "same difference," one is perfectly secure and is what everyone uses -- the other gives root privileges to everyone. In particular, no one uses mode 777 on /run because it would lead to exactly the exploit that this issue describes:

  1. I run /etc/init.d/tinyproxy start to start the daemon
  2. tinyproxy drops privileges and creates a PID filed owned by the tinyproxy user.
  3. There is an exploit that allows an attacker to gain control of tinyproxy.
  4. Fortunately, since tinyproxy is running as the tinyproxy user, the attacker can't do much damage.
  5. The attacker writes 1 in the tinyproxy PID file, which he owns.
  6. I run /etc/init.d/tinyproxy stop to stop the daemon and investigate.
  7. Oops, the machine reboots because I just killed PID 1.

orlitzky commented Jul 28, 2017

755 and 777 are not "same difference," one is perfectly secure and is what everyone uses -- the other gives root privileges to everyone. In particular, no one uses mode 777 on /run because it would lead to exactly the exploit that this issue describes:

  1. I run /etc/init.d/tinyproxy start to start the daemon
  2. tinyproxy drops privileges and creates a PID filed owned by the tinyproxy user.
  3. There is an exploit that allows an attacker to gain control of tinyproxy.
  4. Fortunately, since tinyproxy is running as the tinyproxy user, the attacker can't do much damage.
  5. The attacker writes 1 in the tinyproxy PID file, which he owns.
  6. I run /etc/init.d/tinyproxy stop to stop the daemon and investigate.
  7. Oops, the machine reboots because I just killed PID 1.
@tb3088

This comment has been minimized.

Show comment
Hide comment
@tb3088

tb3088 Jul 28, 2017

I meant Amazon Linux runs 755 as well. It doesn't really matter if /var/run is root only or not. Say your pid file gets written to /var/run/tinyproxy/myport.pid. You have the exact same problem. The issue isn't who writes the pid file. The issue is don't blindly trust contents. That said, something like this is ingrained in pretty much any sysadmin.
kill -9 cat /var/run/prog.pid

I think the only real solution is TinyProxy, nay all programs simply do not write their pid files, ever. It's the caller's responsibility to do it. If said caller can't write to the system pid directory, too bad.

tb3088 commented Jul 28, 2017

I meant Amazon Linux runs 755 as well. It doesn't really matter if /var/run is root only or not. Say your pid file gets written to /var/run/tinyproxy/myport.pid. You have the exact same problem. The issue isn't who writes the pid file. The issue is don't blindly trust contents. That said, something like this is ingrained in pretty much any sysadmin.
kill -9 cat /var/run/prog.pid

I think the only real solution is TinyProxy, nay all programs simply do not write their pid files, ever. It's the caller's responsibility to do it. If said caller can't write to the system pid directory, too bad.

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Jul 28, 2017

o+rwx means writable by everyone, the o doesn't mean owner =)

orlitzky commented Jul 28, 2017

o+rwx means writable by everyone, the o doesn't mean owner =)

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Jul 28, 2017

If you can't trust the contents of the PID file, why have it at all? It's useless, like I mentioned over in issue #24. The problem with that is that PID files are the standard way to stop, reload, flush -- whatever -- a running daemon. There is no way to stop a running instance of tinyproxy without knowing its PID. If you can't get that PID from the PID file -- where can your init script get it from?

orlitzky commented Jul 28, 2017

If you can't trust the contents of the PID file, why have it at all? It's useless, like I mentioned over in issue #24. The problem with that is that PID files are the standard way to stop, reload, flush -- whatever -- a running daemon. There is no way to stop a running instance of tinyproxy without knowing its PID. If you can't get that PID from the PID file -- where can your init script get it from?

@rofl0r

This comment has been minimized.

Show comment
Hide comment
@rofl0r

rofl0r Jul 29, 2017

Contributor

since your init system starts the process, shouldn't it take care of the bookkeeping ?

Contributor

rofl0r commented Jul 29, 2017

since your init system starts the process, shouldn't it take care of the bookkeeping ?

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Jul 29, 2017

@rofl0r you seem to be suggesting that a PID file isn't necessary at all. Disregarding the fact that all daemons have used a PID file for 40 years and the practical problems of determining the PID after a fork -- right now, tinyproxy is writing a vulnerable PID file.

If the maintainers decide to fix the vulnerability by eliminating the PID file entirely (and making tinyproxy impossible to use with e.g. sysvinit), then so be it. But so long as it writes the PID file, it should do so securely.

orlitzky commented Jul 29, 2017

@rofl0r you seem to be suggesting that a PID file isn't necessary at all. Disregarding the fact that all daemons have used a PID file for 40 years and the practical problems of determining the PID after a fork -- right now, tinyproxy is writing a vulnerable PID file.

If the maintainers decide to fix the vulnerability by eliminating the PID file entirely (and making tinyproxy impossible to use with e.g. sysvinit), then so be it. But so long as it writes the PID file, it should do so securely.

@rofl0r

This comment has been minimized.

Show comment
Hide comment
@rofl0r

rofl0r Jul 29, 2017

Contributor

indeed. i'm pondering the possibility of just removing the usage of the pid file altogether, together with the logfile, which imo should just be printed to stdout (and no daemonizing being done either), since at least in the init implemenation i'm using, runit automatically takes care of all that, and i suspect other init systems have similar possibilities even though i'm not very familiar with them. the only init-helper functionality provided directly in any service could thus (theoretically) consist of a simple signal handler which reaps all process children when a shutdown is signaled.

Contributor

rofl0r commented Jul 29, 2017

indeed. i'm pondering the possibility of just removing the usage of the pid file altogether, together with the logfile, which imo should just be printed to stdout (and no daemonizing being done either), since at least in the init implemenation i'm using, runit automatically takes care of all that, and i suspect other init systems have similar possibilities even though i'm not very familiar with them. the only init-helper functionality provided directly in any service could thus (theoretically) consist of a simple signal handler which reaps all process children when a shutdown is signaled.

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Jul 29, 2017

SystemD and OpenRC can do something similar, but you can't count on every init system or platform supporting it. For example, the first sentence of the tinyproxy webpage is "Tinyproxy is a light-weight HTTP/HTTPS proxy daemon for POSIX operating systems," and systemd is explicitly not POSIX (it only works on relatively new Linux systems).

If your daemon doesn't background itself and doesn't write a PID file, then the init system needs to,

  1. Implement a special supervisor process that knows how to background your program.
  2. Implement bookkeeping to track the PID of your process after it goes into the background, and any time it changes due to e.g. a reload.
  3. Also support the PID file way of doing things, because most software still does that, and every book and class on UNIX programming teaches it that way.
  4. Ship a new init script to every software package in existence.

Maybe it's a noble goal but the scope is a lot bigger than what I'm prepared to take on. For now I just want to eliminate the risk to people using the PID file.

orlitzky commented Jul 29, 2017

SystemD and OpenRC can do something similar, but you can't count on every init system or platform supporting it. For example, the first sentence of the tinyproxy webpage is "Tinyproxy is a light-weight HTTP/HTTPS proxy daemon for POSIX operating systems," and systemd is explicitly not POSIX (it only works on relatively new Linux systems).

If your daemon doesn't background itself and doesn't write a PID file, then the init system needs to,

  1. Implement a special supervisor process that knows how to background your program.
  2. Implement bookkeeping to track the PID of your process after it goes into the background, and any time it changes due to e.g. a reload.
  3. Also support the PID file way of doing things, because most software still does that, and every book and class on UNIX programming teaches it that way.
  4. Ship a new init script to every software package in existence.

Maybe it's a noble goal but the scope is a lot bigger than what I'm prepared to take on. For now I just want to eliminate the risk to people using the PID file.

@tb3088

This comment has been minimized.

Show comment
Hide comment
@tb3088

tb3088 Jul 29, 2017

making tinyproxy impossible to use with e.g. sysvinit

say what? There's pidof, pgrep, and other LSB programs to bear. It's foolish to rely on the contents of a pidfile and just because root wrote it doen't mean it's safe. Less so these days but some programs liked to put multiple lines in their pidfiles (eg. the command-line args when it was invoked) which obviously screw with the kill command if just consumed naively.

Systemd is the proud recipient of a Pwnie and should not be considered an init system at all, but rather a built-in hacker's delight to all stupid enough to run it. (seriously, read the list of insane bugs that project is known for - but enough OT).

re;

  1. look at daemon() in init.d/functions (or whatever your OS uses)
  2. reloads don't change the parent process. The children can/should die and be restarted.
  3. well every book was written 30+ years ago where such things as "shoot I might HUP/TERM my pid=1" was not considered as likely eventuality, and two if you were dumb enough to nuke yourself, you deserved the consequences. Unix is not written to be forgiving of silly actions.
  4. Init scripts are the domain of the OS vendor. The mechanisms neigh the entire subsystem changes sometimes dramatically and it's not Tiny's responsibility to track nor worry about what they do nor how they do it.

Tiny simply needs to focus on not doing something stupid (like running as Root forever or failing to give up privs for lack of error handling) and writing PID files is not important and again OS dependent. The correct answer is, don't do it and let the OS packager do his proper duty.

tb3088 commented Jul 29, 2017

making tinyproxy impossible to use with e.g. sysvinit

say what? There's pidof, pgrep, and other LSB programs to bear. It's foolish to rely on the contents of a pidfile and just because root wrote it doen't mean it's safe. Less so these days but some programs liked to put multiple lines in their pidfiles (eg. the command-line args when it was invoked) which obviously screw with the kill command if just consumed naively.

Systemd is the proud recipient of a Pwnie and should not be considered an init system at all, but rather a built-in hacker's delight to all stupid enough to run it. (seriously, read the list of insane bugs that project is known for - but enough OT).

re;

  1. look at daemon() in init.d/functions (or whatever your OS uses)
  2. reloads don't change the parent process. The children can/should die and be restarted.
  3. well every book was written 30+ years ago where such things as "shoot I might HUP/TERM my pid=1" was not considered as likely eventuality, and two if you were dumb enough to nuke yourself, you deserved the consequences. Unix is not written to be forgiving of silly actions.
  4. Init scripts are the domain of the OS vendor. The mechanisms neigh the entire subsystem changes sometimes dramatically and it's not Tiny's responsibility to track nor worry about what they do nor how they do it.

Tiny simply needs to focus on not doing something stupid (like running as Root forever or failing to give up privs for lack of error handling) and writing PID files is not important and again OS dependent. The correct answer is, don't do it and let the OS packager do his proper duty.

@tb3088

This comment has been minimized.

Show comment
Hide comment
@tb3088

tb3088 Jul 29, 2017

I haven't checked, but suspect Tiny doesn't check to see if the log file is a link. Personally I prefer the behavior of If Exist and not a file (optional; or not owned by my EUID), remove, recreate; or bail on logging to that file. Writing to syslog() is of course an expected feature.

tb3088 commented Jul 29, 2017

I haven't checked, but suspect Tiny doesn't check to see if the log file is a link. Personally I prefer the behavior of If Exist and not a file (optional; or not owned by my EUID), remove, recreate; or bail on logging to that file. Writing to syslog() is of course an expected feature.

@orlitzky orlitzky changed the title from tinyproxy should create its PID file before dropping privileges to tinyproxy should create its PID file before dropping privileges (CVE-2017-11747) Aug 1, 2017

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Aug 1, 2017

@tb3088 pidof returns multiple PIDs, and so can pgrep. Which process should you kill if you get back five PIDs? You want to kill the one that the init script started, but there's no way to tell which one that is.

If you really think it can be done, just write me an init script that doesn't trust the contents of the PID file. I don't think you'll be able to, and attempting to do it will reveal the inherent difficulties better than arguing with me will.

orlitzky commented Aug 1, 2017

@tb3088 pidof returns multiple PIDs, and so can pgrep. Which process should you kill if you get back five PIDs? You want to kill the one that the init script started, but there's no way to tell which one that is.

If you really think it can be done, just write me an init script that doesn't trust the contents of the PID file. I don't think you'll be able to, and attempting to do it will reveal the inherent difficulties better than arguing with me will.

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Aug 21, 2017

Any comment from upstream? Since this is a minor issue, I don't want to sit on the CVE announcement forever. I'll probably announce it within the week if no one asks me not to.

orlitzky commented Aug 21, 2017

Any comment from upstream? Since this is a minor issue, I don't want to sit on the CVE announcement forever. I'll probably announce it within the week if no one asks me not to.

@obnoxxx

This comment has been minimized.

Show comment
Hide comment
@obnoxxx

obnoxxx Aug 24, 2017

Member

Hi @orlitzky , @tb3088 , @rofl0r . Thanks for your thoughts.
I have been offline for a while. Need to read up on these lengthy threads...

Member

obnoxxx commented Aug 24, 2017

Hi @orlitzky , @tb3088 , @rofl0r . Thanks for your thoughts.
I have been offline for a while. Need to read up on these lengthy threads...

@obnoxxx

This comment has been minimized.

Show comment
Hide comment
@obnoxxx

obnoxxx Aug 24, 2017

Member

@orlitzky explicitly, please let me read up on this and come back to you soon.

Member

obnoxxx commented Aug 24, 2017

@orlitzky explicitly, please let me read up on this and come back to you soon.

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Aug 24, 2017

@obnoxxx gotcha, no problem

orlitzky commented Aug 24, 2017

@obnoxxx gotcha, no problem

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky commented Nov 15, 2017

Ping =)

obnoxxx added a commit to obnoxxx/tinyproxy that referenced this issue Nov 16, 2017

Fix CVE-2017-11747: Create PID file before dropping privileges.
Resolves #106

Signed-off-by: Michael Adam <obnox@samba.org>
@obnoxxx

This comment has been minimized.

Show comment
Hide comment
@obnoxxx

obnoxxx Nov 16, 2017

Member

@orlitzky pong. Thanks for the ping... Created PR #120 for addressing this.

Member

obnoxxx commented Nov 16, 2017

@orlitzky pong. Thanks for the ping... Created PR #120 for addressing this.

@obnoxxx

This comment has been minimized.

Show comment
Hide comment
@obnoxxx

obnoxxx Nov 16, 2017

Member

The Patch of PR #120 has been merged.

Member

obnoxxx commented Nov 16, 2017

The Patch of PR #120 has been merged.

@tb3088

This comment has been minimized.

Show comment
Hide comment
@tb3088

tb3088 Nov 16, 2017

I'm not convinced. create_file_safely() doesn't look safe at all.
Why do we care if there is an existing file and choose to open it? An existing PID file by definition is garbage. The only correct response is to erase it. And it should NEVER follow symlinks. What if I symlink the pid file to /etc/passwd?

Or heck, what if I name my pidFile something dangerous?
That this is now doing dangerous things as root is just begging for calamity, malicious or not!

So SSHD blindly follows symlinks and truncates the destination file. Oops! I copied /etc/passwd to a separate file, symlinked to it from /var/run and hilarity ensued.

tb3088 commented Nov 16, 2017

I'm not convinced. create_file_safely() doesn't look safe at all.
Why do we care if there is an existing file and choose to open it? An existing PID file by definition is garbage. The only correct response is to erase it. And it should NEVER follow symlinks. What if I symlink the pid file to /etc/passwd?

Or heck, what if I name my pidFile something dangerous?
That this is now doing dangerous things as root is just begging for calamity, malicious or not!

So SSHD blindly follows symlinks and truncates the destination file. Oops! I copied /etc/passwd to a separate file, symlinked to it from /var/run and hilarity ensued.

@obnoxxx

This comment has been minimized.

Show comment
Hide comment
@obnoxxx

obnoxxx Nov 16, 2017

Member

I'm not convinced. create_file_safely() doesn't look safe at all.

Fair point, but that's a different issue, right?
The point of this issue is addressed, now let's see
what we can do to make pidfile creation as root safer?

Let's see:

Why do we care if there is an existing file and choose to open it? An existing PID file by definition is garbage. The only correct response is to erase it.

That sounds fair.

And it should NEVER follow symlinks. What if I symlink the pid file to /etc/passwd?

Ok. So the file exists and is a symlink, bail out.

Or heck, what if I name my pidFile something dangerous?

Well that's misconfiguration then. We can never fully protect from that, right?

That this is now doing dangerous things as root is just begging for calamity, malicious or not!

So SSHD blindly follows symlinks and truncates the destination file. Oops! I copied /etc/passwd to a separate file, symlinked to it from /var/run and hilarity ensued.

Sorry you lost me, what's sshd got to do with it?

Any more things?

Member

obnoxxx commented Nov 16, 2017

I'm not convinced. create_file_safely() doesn't look safe at all.

Fair point, but that's a different issue, right?
The point of this issue is addressed, now let's see
what we can do to make pidfile creation as root safer?

Let's see:

Why do we care if there is an existing file and choose to open it? An existing PID file by definition is garbage. The only correct response is to erase it.

That sounds fair.

And it should NEVER follow symlinks. What if I symlink the pid file to /etc/passwd?

Ok. So the file exists and is a symlink, bail out.

Or heck, what if I name my pidFile something dangerous?

Well that's misconfiguration then. We can never fully protect from that, right?

That this is now doing dangerous things as root is just begging for calamity, malicious or not!

So SSHD blindly follows symlinks and truncates the destination file. Oops! I copied /etc/passwd to a separate file, symlinked to it from /var/run and hilarity ensued.

Sorry you lost me, what's sshd got to do with it?

Any more things?

@tb3088

This comment has been minimized.

Show comment
Hide comment
@tb3088

tb3088 Nov 16, 2017

an earlier version of my comment mentioned SSHd and Apache as examples to perhaps mimic. Until I looked at SSHd which does bad things.

tb3088 commented Nov 16, 2017

an earlier version of my comment mentioned SSHd and Apache as examples to perhaps mimic. Until I looked at SSHd which does bad things.

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Nov 16, 2017

@obnoxxx thanks!

@tb3088

An existing PID file by definition is garbage.

Well, it indicates that something is wrong. Maybe you tried to start two separate tinyproxy instances and forgot to give the second one a different pid file name. Either overwriting the existing file or throwing an error (telling the admin to check and possibly clean up the existing file) sound reasonable to me.

And it should NEVER follow symlinks. What if I symlink the pid file to /etc/passwd?

The theme of this vulnerability is that the pid file (and thus its parent directory) must not be writable by anyone other than root. If root creates a symlink to /etc/passwd, then he's going to get what he asked for. If anyone else tries, they will fail because the pid file will be located in a root-owned directory like /run.

Or heck, what if I name my pidFile something dangerous?

If you want to hose your own system as root, no one's going to stop you. But only root can name the pid file.

orlitzky commented Nov 16, 2017

@obnoxxx thanks!

@tb3088

An existing PID file by definition is garbage.

Well, it indicates that something is wrong. Maybe you tried to start two separate tinyproxy instances and forgot to give the second one a different pid file name. Either overwriting the existing file or throwing an error (telling the admin to check and possibly clean up the existing file) sound reasonable to me.

And it should NEVER follow symlinks. What if I symlink the pid file to /etc/passwd?

The theme of this vulnerability is that the pid file (and thus its parent directory) must not be writable by anyone other than root. If root creates a symlink to /etc/passwd, then he's going to get what he asked for. If anyone else tries, they will fail because the pid file will be located in a root-owned directory like /run.

Or heck, what if I name my pidFile something dangerous?

If you want to hose your own system as root, no one's going to stop you. But only root can name the pid file.

@tb3088

This comment has been minimized.

Show comment
Hide comment
@tb3088

tb3088 Nov 16, 2017

my point is checking for pid files is the wrapping script's responsibility. If exist, check for running program, if not delete it, then invoke TP binary. But as far as teh binary is concerned, if a file exists, log the error, don't do anything and definitely don't follow anything.

Predicating the program's "safe operation" on the pid_dir being root only is idiotic. Dont' write a pid file at all and this whole mess goes away. Not to mention TP should never be run as root in the first place. It should ~never be configured to listen to a low port. If you need it to, then use xinetd or iptables to redirect packets.

The program is mightily untrustworthy as it is. Introducing more vulnerability is not the right thing to do.

tb3088 commented Nov 16, 2017

my point is checking for pid files is the wrapping script's responsibility. If exist, check for running program, if not delete it, then invoke TP binary. But as far as teh binary is concerned, if a file exists, log the error, don't do anything and definitely don't follow anything.

Predicating the program's "safe operation" on the pid_dir being root only is idiotic. Dont' write a pid file at all and this whole mess goes away. Not to mention TP should never be run as root in the first place. It should ~never be configured to listen to a low port. If you need it to, then use xinetd or iptables to redirect packets.

The program is mightily untrustworthy as it is. Introducing more vulnerability is not the right thing to do.

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Nov 17, 2017

Dont' write a pid file at all and this whole mess goes away.

It's your lucky day:

# If not specified, no pidfile will be written.
#
#PidFile "@localstatedir@/run/tinyproxy/tinyproxy.pid"

You are welcome to start tinyproxy as an unprivileged user and forego the PID file if you have some other means of stopping it. But for the people using init scripts with PID files, things are now a tiny bit safer.

orlitzky commented Nov 17, 2017

Dont' write a pid file at all and this whole mess goes away.

It's your lucky day:

# If not specified, no pidfile will be written.
#
#PidFile "@localstatedir@/run/tinyproxy/tinyproxy.pid"

You are welcome to start tinyproxy as an unprivileged user and forego the PID file if you have some other means of stopping it. But for the people using init scripts with PID files, things are now a tiny bit safer.

@tb3088

This comment has been minimized.

Show comment
Hide comment
@tb3088

tb3088 Nov 17, 2017

excellent. So now we're down to you only need root to bind to a low port. Since .../run/tinyproxy/ should be owned by the post-dropprivs() account. and even if you have nasties in that directory it can't do bad things.

tb3088 commented Nov 17, 2017

excellent. So now we're down to you only need root to bind to a low port. Since .../run/tinyproxy/ should be owned by the post-dropprivs() account. and even if you have nasties in that directory it can't do bad things.

@obnoxxx obnoxxx closed this in #120 Feb 9, 2018

obnoxxx added a commit that referenced this issue Feb 9, 2018

Fix CVE-2017-11747: Create PID file before dropping privileges.
Resolves #106

Signed-off-by: Michael Adam <obnox@samba.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment