Bug #1335
closedsuricata option --pidfile overwrites any file
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
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.
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.
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.
Updated by Jason Ish over 7 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.