[PATCH 4/6] dtprobed: harden helper ioctl validation

Reject unterminated fixed-size module names and module names that would
be unsafe as pathname components before using helper data.  Reuse the same
component-safety predicate used when composing probe specs, and handle
userdata allocation failure before dereferencing it.

Orabug: 39374493
Signed-off-by: Kris Van Hees <[email protected]>
---
 dtprobed/dof_stash.c | 27 +++++++++++++++------------
 dtprobed/dof_stash.h |  2 ++
 dtprobed/dtprobed.c  | 22 ++++++++++++++++++++++
 3 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/dtprobed/dof_stash.c b/dtprobed/dof_stash.c
index 4c7a95cd..891d7a69 100644
--- a/dtprobed/dof_stash.c
+++ b/dtprobed/dof_stash.c
@@ -230,6 +230,17 @@ make_provpid_name(const char *prov, pid_t pid)
 	return ret;
 }
 
+/*
+ * Ban "." and ".." as probe description components. Components are also not
+ * allowed to contain any '/' character.
+ */
+int
+is_component_unsafe(const char *s)
+{
+	return strcmp(s, ".") == 0 || strcmp(s, "..") == 0 ||
+	       strchr(s, '/') != NULL;
+}
+
 /*
  * Compose a full probespec's name from pieces.
  */
@@ -240,19 +251,11 @@ make_probespec_name(const char *prov, const char *mod, const char *fn,
 	char *ret;
 
 	/*
-	 * Ban "." and ".." as probe description components, as well as any
-	 * components with a '/' character.  Since the components are used in
-	 * the creation of paths that will be written to, any of these cases
-	 * can be unsafe.
+	 * Since probe description components are used in the creation of paths
+	 * that will be written to, make sure their content is safe.
 	 */
-	if (strcmp(prov, ".") == 0 || strcmp(prov, "..") == 0 ||
-	    strchr(prov, '/') != NULL ||
-	    strcmp(mod, ".") == 0 || strcmp(mod, "..") == 0 ||
-	    strchr(mod, '/') != NULL ||
-	    strcmp(fn, ".") == 0 || strcmp(fn, "..") == 0 ||
-	    strchr(fn, '/') != NULL ||
-	    strcmp(prb, ".") == 0 || strcmp(prb, "..") == 0 ||
-	    strchr(prb, '/') != NULL)
+	if (is_component_unsafe(prov) || is_component_unsafe(mod) ||
+	    is_component_unsafe(fn) || is_component_unsafe(prb))
 		return NULL;
 
 	if (asprintf(&ret, "%s:%s:%s:%s", prov, mod, fn, prb) < 0) {
diff --git a/dtprobed/dof_stash.h b/dtprobed/dof_stash.h
index 32b5eb52..e72671af 100644
--- a/dtprobed/dof_stash.h
+++ b/dtprobed/dof_stash.h
@@ -21,6 +21,8 @@ typedef struct dof_parsed_list {
 
 int dof_stash_init(const char *statedir);
 
+int is_component_unsafe(const char *s);
+
 int dof_stash_push_parsed(dt_list_t *accum, dof_parsed_t *parsed);
 int dof_stash_write_parsed(pid_t pid, dev_t dev, ino_t ino, dt_list_t *accum);
 void dof_stash_free(dt_list_t *accum);
diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
index 9ce941ee..c85a6617 100644
--- a/dtprobed/dtprobed.c
+++ b/dtprobed/dtprobed.c
@@ -668,6 +668,16 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
 	int gen;
 	usdt_data_t data;
 
+	/* If userdata == NULL, we were not able to allocate memory for it. */
+	if (userdata == NULL) {
+		fuse_log(FUSE_LOG_ERR, "%i: dtprobed: %s\n", pid,
+				 "out of memory allocating userdata\n");
+		if (fuse_reply_err(req, ENOMEM) < 0)
+			fuse_log(FUSE_LOG_ERR, "%i: dtprobed: %s\n", pid,
+				 "cannot send error to ioctl caller\n");
+		return;
+	}
+
 	/*
 	 * We can just ignore FUSE_IOCTL_COMPAT: the 32-bit and 64-bit versions
 	 * of the DOF structures are intentionally identical.
@@ -729,7 +739,19 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
 				 errmsg, sizeof(dof_helper_t), in_bufsz);
 			goto fuse_err;
 		}
+
 		memcpy(&userdata->dh, in_buf, sizeof(dof_helper_t));
+		if (memchr(userdata->dh.dofhp_mod, 0, DTRACE_MODNAMELEN) == NULL) {
+			fuse_log(FUSE_LOG_ERR, "%i: dtprobed: "
+				 "unterminated module name\n", pid);
+			goto fuse_err;
+		}
+		if (is_component_unsafe(userdata->dh.dofhp_mod)) {
+			fuse_log(FUSE_LOG_ERR, "%i: dtprobed: "
+				 "unsafe characters in module name %s\n",
+				 pid, userdata->dh.dofhp_mod);
+			goto fuse_err;
+		}
 
 		in.iov_base = (void *) userdata->dh.dofhp_dof;
 		in.iov_len = sizeof(dof_hdr_t);
-- 
2.47.3