Project

General

Profile

Actions

Bug #1335

closed

suricata option --pidfile overwrites any file

Added by Laura Brodie almost 10 years ago. Updated almost 8 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

The suricata option --pidfile could overwrite any file either mistakenly or maliciously. These both overwrite the pre-existing file with suricata's pid.

sudo suricata -c /etc/suricata/suricata.yaml -i eth0 --pidfile a_file_i_needed_to_keep.txt
sudo suricata -c /etc/suricata/suricata.yaml -i eth0 --pidfile ../../bin/gunzip

I was afraid to try this one - who know what would happen:

sudo suricata -c /etc/suricata/suricata.yaml -i eth0 --pidfile ../../run/samba/samba.pid

or

nmbd.pd, smbd.pid, winbindd.pid
Actions #1

Updated by Victor Julien almost 10 years ago

  • Status changed from New to Assigned
  • Assignee set to Jason Ish

We should probably just error out if the file exists.

Actions #2

Updated by Jason Ish almost 10 years ago

Note that this problem all exists when daemonizing (so prior to the modification that allowed a pidfile to be created when running in the foreground).

I guess the idea here is that someone might have access to start suricata with sudo but not have access to edit the configuration file? Otherwise the user could use sudo to edit the configuration, restart suricata and have the same affect.

Erroring out of the file exists is one option, leaving the user to clean it up. It might cause issues in automated restart scenarios to catch crashes where Suricata was not able to completely cleanup after itself.

I took a look at a couple other applications that write pid files. They also overwrote their pid file, but instead of allowing the user to choose the filename, they used a predefined filename only letting the user choose the directory. This does provide some safety from pointing it at an arbitrary file, and it could let us keep the convenience of letting Suricata overwrite the file but it would be a change in the pid file handling, plus command line and configuration file changes.

Actions #3

Updated by Andreas Herz about 8 years ago

  • Target version set to TBD
Actions #4

Updated by Jason Ish about 8 years ago

I wonder how much of a problem this really is. There are other services started on on a regular Linux box that have their PID files specified on the command line just like this, I assume they suffer the same issue, which ultimately comes down to trusting the people you let start Suricata with sudo right?

The other option is to lock it down to --localstatedir, but allow the user to set the filename for the case when multiple instances are being run.

Actions #5

Updated by Jason Ish almost 8 years ago

  • Target version changed from TBD to 70
Actions #6

Updated by Jason Ish almost 8 years ago

  • Status changed from Assigned to Closed
  • Target version changed from 70 to 4.0beta1

Fixed by failing to start on the existence of the pid file.

See:
https://github.com/inliniac/suricata/pull/2573

Actions

Also available in: Atom PDF