From 30e19216bfadc0148f347cb0a42fd5e2016e6269 Mon Sep 17 00:00:00 2001
From: Tobias Oetiker <tobi@oetiker.ch>
Date: Tue, 30 Jun 2026 23:34:40 +0200
Subject: [PATCH] Fix symlink-following chown of pid file in daemon mode (CWE-59)
         .
 When mrtg is started as root in daemon mode (--daemon --user), it
 created the pid file and chown'ed it to the target user *before*
 dropping privileges. Both create_pid()'s `-e`/`open(">...")` and the
 subsequent `chown` follow symlinks, so a local attacker who can
 pre-place a symlink at the pid path (e.g. a pid file in a writable
 directory) could make root chown an arbitrary existing file to the
 daemon user, or create a root-owned file at an attacker-chosen path.
 .
 Rather than reorder the privilege drop (which would break the common
 case of a root-owned pid directory, where the unprivileged daemon
 cannot create the file itself), keep creating the file while
 privileged but do it safely:
 .
 - create_pid() refuses symlinks and creates the file with
   O_WRONLY|O_CREAT|O_EXCL, closing the symlink-follow / TOCTOU window.
 - It chowns the open filehandle (fchown) instead of the path, so the
   ownership change cannot be redirected through a swapped-in symlink.
   The caller no longer does a separate path-based chown.
 - demonize_me()'s later pid write refuses symlinks too.
Origin: https://github.com/oetiker/mrtg/commit/30e1921
Bug: https://github.com/oetiker/mrtg/pull/123
Bug-Debian: https://bugs.debian.org/1144393
Index: mrtg/bin/mrtg
===================================================================
--- mrtg.orig/bin/mrtg
+++ mrtg/bin/mrtg
@@ -252,9 +252,11 @@ sub main {
 
     # Run as a daemon, specified on command line (required for FHS compliant daemon)
     if (defined $opts{"daemon"}) {
-	# Create a pidfile, then chown it so we can use it once we change user
-	&create_pid($pidfile);
-	chown $uid, $gid, $pidfile;
+	# Create the pidfile securely and, while still privileged, hand it to
+	# the user we are about to become so the daemon can update it later.
+	# create_pid refuses symlinks and chowns the open handle (not the path),
+	# so a hostile pid path cannot be used to chown an arbitrary file.
+	&create_pid($pidfile, $uid, $gid);
     }
 
     ($(,$)) = ($gid,$gid) ;
Index: mrtg/lib/mrtg2/MRTG_lib.pm
===================================================================
--- mrtg.orig/lib/mrtg2/MRTG_lib.pm
+++ mrtg/lib/mrtg2/MRTG_lib.pm
@@ -16,6 +16,7 @@ package MRTG_lib;
 
 require 5.005;
 use strict;
+use Fcntl qw(O_WRONLY O_CREAT O_EXCL);
 use vars qw($OS $SL $PS @EXPORT @ISA $VERSION %timestrpospattern);
 
 
@@ -1233,14 +1234,31 @@ sub expistr ($) {
     return "$wday, $mday $month ".($year+1900)." $hour:$min:$sec GMT";
 }
 
-sub create_pid ($) {
-    my $pidfile = shift;
+sub create_pid ($;$$) {
+    my ($pidfile, $uid, $gid) = @_;
     return if ($OS eq 'NT' );
+
+    # Security: refuse to operate on a symlink. When mrtg is started as root
+    # in daemon mode with a writable pid path, an attacker who pre-places a
+    # symlink here could otherwise make us create or chown an arbitrary file
+    # (CWE-59). A plain stat/-e on the path would follow the link, so check
+    # the link itself first.
+    if (-l $pidfile) {
+        warn "refusing to use pid file $pidfile: it is a symbolic link\n";
+        return;
+    }
     return if -e $pidfile;
-    if ( open(PIDFILE,">$pidfile")) {
-         close PIDFILE;
+
+    # O_CREAT|O_EXCL creates the file atomically and fails if anything
+    # (including a symlink that was raced in after the check above) already
+    # exists at the path, closing the symlink-follow / TOCTOU window.
+    if ( sysopen(my $fh, $pidfile, O_WRONLY|O_CREAT|O_EXCL, 0644) ) {
+         # chown the open handle (fchown) rather than the path, so the
+         # ownership change cannot be redirected through a swapped-in symlink.
+         chown $uid, $gid, $fh if defined $uid and defined $gid;
+         close $fh;
     } else {
-         warn "cannot write to $pidfile: $!\n";
+         warn "cannot create pid file $pidfile: $!\n";
     }
 }
 
@@ -1286,7 +1304,9 @@ sub demonize_me ($) {
             } else {
                 if (defined $pidfile){
                    $main::Cleanfile3 = $pidfile;
-                   if (open(PIDFILE,">$pidfile")) {
+                   if (-l $pidfile) {
+                        warn "refusing to write pid file $pidfile: it is a symbolic link\n";
+                   } elsif (open(PIDFILE,">$pidfile")) {
                         print PIDFILE "$$\n";
                         close PIDFILE;
                    } else {
