daemon: fix arbitrary file deletion in the privileged process

The `asroot_ctl_cleanup()` handler reads an arbitrary path from the
unprivileged process and deletes it. Instead, introduce
`asroot_ctl_cleanup_lock()` to only clean the lock and
`asroot_ctl_cleanup()` cleans the socket.

Fix #772

diff --git a/NEWS b/NEWS
index 2838bcf40eacf04d41a4994f828111c987a0a305..69abd7e4a7b8320c9bb1058649a0566c2c7f585c 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,7 @@ lldpd (1.0.21)
    + Add "configure lldp portdescription-source" to choose how to populate port description (#763)
  * Fix:
    + Fix path traversal vulnerabilities in the privileged process (#773, #774)
+   + Fix arbitrary file deletion in the privileged process (#772)
    + Fix accuracy of Dot3 MAU types advertised and add support for 200G and 400G (#771)
 
 lldpd (1.0.20)

diff --git a/src/daemon/lldpd.c b/src/daemon/lldpd.c
index 736a149cd1c576aba2d3996c409f5d6dc79ec12e..a6b0728ada2fd424afa5717466da9a589a95e75a 100644 (file)
--- a/src/daemon/lldpd.c
+++ b/src/daemon/lldpd.c
@@ -1317,19 +1317,15 @@ lldpd_loop(struct lldpd *cfg)
 static void
 lldpd_exit(struct lldpd *cfg)
 {
-       char *lockname = NULL;
        struct lldpd_hardware *hardware, *hardware_next;
        log_debug("main", "exit lldpd");
 
        TAILQ_FOREACH (hardware, &cfg->g_hardware, h_entries)
                lldpd_send_shutdown(hardware);
 
-       if (asprintf(&lockname, "%s.lock", cfg->g_ctlname) != -1) {
-               priv_ctl_cleanup(lockname);
-               free(lockname);
-       }
        close(cfg->g_ctl);
-       priv_ctl_cleanup(cfg->g_ctlname);
+       priv_ctl_cleanup_lock();
+       priv_ctl_cleanup();
        log_debug("main", "cleanup hardware information");
        for (hardware = TAILQ_FIRST(&cfg->g_hardware); hardware != NULL;
             hardware = hardware_next) {
@@ -1918,9 +1914,9 @@ lldpd_main(int argc, char *argv[], char *envp[])
 
        log_debug("main", "initialize privilege separation");
 #ifdef ENABLE_PRIVSEP
-       priv_init(PRIVSEP_CHROOT, ctl, uid, gid);
+       priv_init(PRIVSEP_CHROOT, ctl, uid, gid, ctlname);
 #else
-       priv_init();
+       priv_init(ctlname);
 #endif
 
        /* Initialization of global configuration */

diff --git a/src/daemon/lldpd.h b/src/daemon/lldpd.h
index 3b7373a88462f28b46de3cf93876f9f149ab5b44..982456bd5b8cdfe8a92f46dcfda70fbbd8d2180f 100644 (file)
--- a/src/daemon/lldpd.h
+++ b/src/daemon/lldpd.h
@@ -193,12 +193,13 @@ int client_handle_client(struct lldpd *cfg,
 
 /* priv.c */
 #ifdef ENABLE_PRIVSEP
-void priv_init(const char *, int, uid_t, gid_t);
+void priv_init(const char *, int, uid_t, gid_t, const char *);
 #else
-void priv_init(void);
+void priv_init(const char *);
 #endif
 void priv_wait(void);
-void priv_ctl_cleanup(const char *ctlname);
+void priv_ctl_cleanup(void);
+void priv_ctl_cleanup_lock(void);
 char *priv_gethostname(void);
 #ifdef HOST_OS_LINUX
 int priv_open(const char *);
@@ -216,6 +217,7 @@ int priv_snmp_socket(struct sockaddr_un *);
 enum priv_cmd {
        PRIV_PING,
        PRIV_DELETE_CTL_SOCKET,
+       PRIV_DELETE_CTL_SOCKET_LOCK,
        PRIV_GET_HOSTNAME,
        PRIV_OPEN,
        PRIV_IFACE_INIT,

diff --git a/src/daemon/priv.c b/src/daemon/priv.c
index d642328e42caf62b93d7777cc69fb1bed127cc28..2266ea9e0f1587da1cc3b47ae63c9a5d4569df2e 100644 (file)
--- a/src/daemon/priv.c
+++ b/src/daemon/priv.c
@@ -76,6 +76,8 @@ int res_init(void);
 static int monitored = -1; /* Child */
 #endif
 
+static char *ctlname = NULL; /* Registered control socket path */
+
 /* Proxies */
 static void
 priv_ping()
@@ -90,13 +92,22 @@ priv_ping()
 
 /* Proxy for ctl_cleanup */
 void
-priv_ctl_cleanup(const char *ctlname)
+priv_ctl_cleanup(void)
 {
-       int rc, len = strlen(ctlname);
+       int rc;
        enum priv_cmd cmd = PRIV_DELETE_CTL_SOCKET;
        must_write(PRIV_UNPRIVILEGED, &cmd, sizeof(enum priv_cmd));
-       must_write(PRIV_UNPRIVILEGED, &len, sizeof(int));
-       must_write(PRIV_UNPRIVILEGED, ctlname, len);
+       priv_wait();
+       must_read(PRIV_UNPRIVILEGED, &rc, sizeof(int));
+}
+
+/* Proxy for ctl_cleanup_lock */
+void
+priv_ctl_cleanup_lock(void)
+{
+       int rc;
+       enum priv_cmd cmd = PRIV_DELETE_CTL_SOCKET_LOCK;
+       must_write(PRIV_UNPRIVILEGED, &cmd, sizeof(enum priv_cmd));
        priv_wait();
        must_read(PRIV_UNPRIVILEGED, &rc, sizeof(int));
 }
@@ -198,21 +209,29 @@ asroot_ping()
 static void
 asroot_ctl_cleanup()
 {
-       int len;
-       char *ctlname;
        int rc = 0;
+       if (ctlname == NULL) {
+               log_warnx("privsep", "no control socket path registered");
+               rc = -1;
+       } else {
+               ctl_cleanup(ctlname);
+       }
+       must_write(PRIV_PRIVILEGED, &rc, sizeof(int));
+}
 
-       must_read(PRIV_PRIVILEGED, &len, sizeof(int));
-       if (len < 0 || len > PATH_MAX) fatalx("privsep", "too large value requested");
-       if ((ctlname = (char *)malloc(len + 1)) == NULL) fatal("privsep", NULL);
-
-       must_read(PRIV_PRIVILEGED, ctlname, len);
-       ctlname[len] = 0;
-
-       ctl_cleanup(ctlname);
-       free(ctlname);
-
-       /* Ack */
+static void
+asroot_ctl_cleanup_lock()
+{
+       char *received_lockname = NULL;
+       int rc = 0;
+       if (ctlname == NULL ||
+           asprintf(&received_lockname, "%s.lock", ctlname) == -1) {
+               log_warnx("privsep", "no control socket path registered");
+               rc = -1;
+       } else {
+               ctl_cleanup(received_lockname);
+               free(received_lockname);
+       }
        must_write(PRIV_PRIVILEGED, &rc, sizeof(int));
 }
 
@@ -385,6 +404,7 @@ struct dispatch_actions {
 
 static struct dispatch_actions actions[] = { { PRIV_PING, asroot_ping },
        { PRIV_DELETE_CTL_SOCKET, asroot_ctl_cleanup },
+       { PRIV_DELETE_CTL_SOCKET_LOCK, asroot_ctl_cleanup_lock },
        { PRIV_GET_HOSTNAME, asroot_gethostname },
 #ifdef HOST_OS_LINUX
        { PRIV_OPEN, asroot_open },
@@ -691,11 +711,16 @@ priv_caps(uid_t uid, gid_t gid)
 
 void
 #ifdef ENABLE_PRIVSEP
-priv_init(const char *chrootdir, int ctl, uid_t uid, gid_t gid)
+priv_init(const char *chrootdir, int ctl, uid_t uid, gid_t gid, const char *ctlname)
 #else
-priv_init(void)
+priv_init(const char *ctlname)
 #endif
 {
+       /* Store the expected control socket path for asroot_ctl_cleanup() */
+       if (ctlname) {
+               ctlname = strdup(ctlname);
+               if (ctlname == NULL) fatal("privsep", NULL);
+       }
 
        int pair[2];