diff mbox

[ovs-dev,v13,3/6] netdev-dpdk: Restrict vhost_sock_dir

Message ID 1461951845-11682-4-git-send-email-aconole@redhat.com
State Accepted
Headers show

Commit Message

Aaron Conole April 29, 2016, 5:44 p.m. UTC
Since the vhost-user sockets directory now comes from the database, it is
possible for any user with database access to program an arbitrary filesystem
location for the sockets directory. This could result in unprivileged users
creating or deleting arbitrary filesystem files by using specially crafted
names. To prevent this, use the introduced ovs_realpath function to resolve
the filesystem location and ensure that it resolves under the ovs_rundir.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
v13:
* Handle the case where vhost-sock-dir is not set in the database

 lib/netdev-dpdk.c    | 31 ++++++++++++++++++++++++-------
 vswitchd/vswitch.xml |  4 +++-
 2 files changed, 27 insertions(+), 8 deletions(-)
diff mbox

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 59ea51d..8706f9f 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2895,6 +2895,9 @@  dpdk_init__(const struct smap *ovs_other_config)
     int argc;
     int err;
     cpu_set_t cpuset;
+#ifndef VHOST_CUSE
+    char *sock_dir_subcomponent;
+#endif
 
     if (!smap_get_bool(ovs_other_config, "dpdk-init", false)) {
         VLOG_INFO("DPDK Disabled - to change this requires a restart.\n");
@@ -2907,15 +2910,29 @@  dpdk_init__(const struct smap *ovs_other_config)
     if (process_vhost_flags("cuse-dev-name", xstrdup("vhost-net"),
                             PATH_MAX, ovs_other_config, &cuse_dev_name)) {
 #else
-    if (process_vhost_flags("vhost-sock-dir", xstrdup(ovs_rundir()),
-                            NAME_MAX, ovs_other_config, &vhost_sock_dir)) {
+    if (process_vhost_flags("vhost-sock-dir", xstrdup(""),
+                            NAME_MAX, ovs_other_config,
+                            &sock_dir_subcomponent)) {
         struct stat s;
-
-        err = stat(vhost_sock_dir, &s);
-        if (err) {
-            VLOG_ERR("vhost-user sock directory '%s' does not exist.",
-                      vhost_sock_dir);
+        if (!strstr(sock_dir_subcomponent, "..")) {
+            vhost_sock_dir = xasprintf("%s/%s", ovs_rundir(),
+                                       sock_dir_subcomponent);
+
+            err = stat(vhost_sock_dir, &s);
+            if (err) {
+                VLOG_ERR("vhost-user sock directory '%s' does not exist.",
+                         vhost_sock_dir);
+            }
+        } else {
+            vhost_sock_dir = xstrdup(ovs_rundir());
+            VLOG_ERR("vhost-user sock directory request '%s/%s' has invalid"
+                     "characters '..' - using %s instead.",
+                     ovs_rundir(), sock_dir_subcomponent, ovs_rundir());
         }
+        free(sock_dir_subcomponent);
+    } else {
+        vhost_sock_dir = xstrdup(ovs_rundir());
+        free(sock_dir_subcomponent);
 #endif
     }
 
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index c36cb59..400428b 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -290,7 +290,9 @@ 
       <column name="other_config" key="vhost-sock-dir"
               type='{"type": "string"}'>
         <p>
-          Specifies the path to the vhost-user unix domain socket files.
+          Specifies the path to the vhost-user unix domain socket files. This
+          path must exist and be a subdirectory tree of the Open vSwitch
+          run directory.
         </p>
         <p>
           Defaults to the working directory of the application. Changing this