[PATCH][pseudo] client: strip trailing slashes when opening an ignored path

[PATCH][pseudo] client: strip trailing slashes when opening an ignored path

[Ross Burton]

The pseudo client path map stores paths that have been sanitised, but
in the ignored-path (PSEUDO_IGNORE_PATHS) codepath for open() calls this
sanitising wasn't performed so it is possible for paths that end with a
trailing slash to be entered.

This then subsequently interacts badly with path manipulation, resulting
in the situation where doing:

  fd = open("/some/path/")
  parent_fd = openat(fd, "../)

results in parent_fd actually pointing at /some/path still.

Solve this by ensuring that any trailing slashes are removed from the
path when adding to the map in the ignore short-circuit.

Also add a test case for this to ensure that it doesn't regress in the
future.

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 Makefile.in         |  1 +
 pseudo_client.c     | 11 ++++++++-
 test/test-openat.c  | 55 +++++++++++++++++++++++++++++++++++++++++++++
 test/test-openat.sh |  7 ++++++
 4 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 test/test-openat.c
 create mode 100755 test/test-openat.sh

diff --git a/Makefile.in b/Makefile.in
index 6eff522..cf13010 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -76,6 +76,7 @@ all: $(LIBPSEUDO) $(PSEUDO) $(PSEUDODB) $(PSEUDOLOG) $(PSEUDO_PROFILE)
 
 test: all | $(BIN) $(LIB) $(LOCALSTATE)
 	$(CC) $(CFLAGS) $(CFLAGS_PSEUDO) -o test/test-rename-fstat test/test-rename-fstat.c
+	$(CC) $(CFLAGS) $(CFLAGS_PSEUDO) -o test/test-openat test/test-openat.c
 	@./run_tests.sh -v
 
 install-lib: $(LIBPSEUDO)
diff --git a/pseudo_client.c b/pseudo_client.c
index f846d54..579db33 100644
--- a/pseudo_client.c
+++ b/pseudo_client.c
@@ -1629,7 +1629,16 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
 	if (op != OP_CHROOT && op != OP_CHDIR && op != OP_CLOSE && op != OP_DUP
 			&& pseudo_client_ignore_path_chroot(path, 0)) {
 		if (op == OP_OPEN) {
-			pseudo_client_path(fd, path);
+			/* Sanitise the path to have no trailing slash as this is convention in the database */
+			char *stripped_path;
+			pathlen = strlen(path);
+			if (pathlen > 2 && (path[pathlen - 1]) == '/') {
+				stripped_path = strndup(path, pathlen-1);
+				pseudo_client_path(fd, stripped_path);
+				free(stripped_path);
+			} else {
+				pseudo_client_path(fd, path);
+			}
 		}
 		/* reenable wrappers */
 		pseudo_magic();
diff --git a/test/test-openat.c b/test/test-openat.c
new file mode 100644
index 0000000..b710285
--- /dev/null
+++ b/test/test-openat.c
@@ -0,0 +1,55 @@
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <limits.h>
+#include <unistd.h>
+#include <string.h>
+
+char *path_of(int fd) {
+    ssize_t len;
+    char proc[PATH_MAX], real[PATH_MAX];
+    snprintf(proc, sizeof(proc), "/proc/self/fd/%d", fd);
+    len = readlink(proc, real, sizeof(real));
+    real[len] = '\0';
+    return strdup(real);
+}
+
+/*
+* Test that recusing up the tree with openat(fd, "../") handles slashes
+* correctly and doesn't end up opening the same directory instead of going up a
+* level.
+*/
+int main () {
+    int fd, dir_fd;
+    struct stat st;
+    ino_t ino;
+    char *path;
+
+    fd = openat(AT_FDCWD, ".", O_DIRECTORY, 0);
+    fstat(fd, &st);
+    ino = st.st_ino;
+
+    while (1) {
+        path = path_of(fd);
+        //puts(path);
+
+        dir_fd = openat(fd, "../", O_DIRECTORY, 0);
+        fstat(dir_fd, &st);
+        if (st.st_ino == ino) {
+            if (strcmp(path, "/") == 0) {
+                //puts("Reached top of tree");
+                return 0;
+            } else {
+                //puts("Recursion failed!");
+                return 1;
+            }
+        }
+
+        free (path);
+        ino = st.st_ino;
+        fd = dir_fd;
+    }
+    return 0;
+}
diff --git a/test/test-openat.sh b/test/test-openat.sh
new file mode 100755
index 0000000..0455586
--- /dev/null
+++ b/test/test-openat.sh
@@ -0,0 +1,7 @@
+#! /bin/sh
+
+# Test with and without paths being ignored. The bug was with paths being ignored.
+
+./test/test-openat
+
+PSEUDO_IGNORE_PATHS=/ ./test/test-openat
-- 
2.25.1

Re: [OE-core] [PATCH][pseudo] client: strip trailing slashes when opening an ignored path

[Richard Purdie]

On Thu, 2021-04-08 at 17:25 +0100, Ross Burton wrote:
> The pseudo client path map stores paths that have been sanitised, but
> in the ignored-path (PSEUDO_IGNORE_PATHS) codepath for open() calls this
> sanitising wasn't performed so it is possible for paths that end with a
> trailing slash to be entered.
> 
> This then subsequently interacts badly with path manipulation, resulting
> in the situation where doing:
> 
>   fd = open("/some/path/")
>   parent_fd = openat(fd, "../)
> 
> results in parent_fd actually pointing at /some/path still.
> 
> Solve this by ensuring that any trailing slashes are removed from the
> path when adding to the map in the ignore short-circuit.
> 
> Also add a test case for this to ensure that it doesn't regress in the
> future.
> 
> Signed-off-by: Ross Burton <ross.burton@arm.com>
> ---
>  Makefile.in         |  1 +
>  pseudo_client.c     | 11 ++++++++-
>  test/test-openat.c  | 55 +++++++++++++++++++++++++++++++++++++++++++++
>  test/test-openat.sh |  7 ++++++
>  4 files changed, 73 insertions(+), 1 deletion(-)
>  create mode 100644 test/test-openat.c
>  create mode 100755 test/test-openat.sh

Merged to the oe-core branch, thanks. Bonus kudos for a new test! :)

Cheers,

Richard