From a3a6f078820576c48bee4a69754396178ac95cb6 Mon Sep 17 00:00:00 2001
From: Jacek Maksymowicz <jacek.maksymowicz@phoenix-rtos.com>
Date: Fri, 16 Feb 2024 16:31:29 +0100
Subject: [PATCH] unistd/dir.c: fix resolve_path behavior on trailing slashes

JIRA: RTOS-782
---
 unistd/dir.c | 79 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 29 deletions(-)

diff --git a/unistd/dir.c b/unistd/dir.c
index 25431794..9de9e4a1 100644
--- a/unistd/dir.c
+++ b/unistd/dir.c
@@ -173,13 +173,14 @@ static int _resolve_abspath(char *path, char *result, int resolve_last_symlink,
 		p = strchrnul(node, '/');
 		const size_t node_len = p - node;
 
-		/* skip trailing slashes to properly detect leaf node */
+		const int is_leaf = (*p == '\0') ? 1 : 0;
 		while (*p == '/') {
 			p += 1;
 		}
 
-		const int is_leaf = (*p == '\0') ? 1 : 0;
-
+		/* TODO: this is too permissive, because it allows creating files (not just directories)
+		 * with a path that has trailing slashes */
+		const int is_leaf_after_slashes = (*p == '\0') ? 1 : 0;
 		/* check for '.' and '..' */
 		if (node_len == 0) { /* multiple '/' */
 			continue;
@@ -202,11 +203,6 @@ static int _resolve_abspath(char *path, char *result, int resolve_last_symlink,
 		r += node_len;
 		*r = '\0';
 
-		if ((is_leaf != 0) && (resolve_last_symlink == 0)) {
-			/* WARN: slight inconsistency: we're not checking if the path actually exists in this case (TODO?) */
-			break;
-		}
-
 		/*
 		 * Number of initial unused `path` bytes that can be used to resolve current node. If link content is
 		 * longer we will not be able to merge it with remaining path and still fit in PATH_MAX buffer
@@ -217,18 +213,29 @@ static int _resolve_abspath(char *path, char *result, int resolve_last_symlink,
 		ssize_t symlink_len = _readlink_abs(result, path, readlink_max_len);
 
 		if (symlink_len < 0) {
-			if (errno == EINVAL) { /* not a symlink */
+			if (symlink_len == -EISDIR) {
 				continue;
 			}
-			else if ((errno == ENOENT) && (is_leaf != 0) && (allow_missing_leaf != 0)) { /* non-esixting leaf */
+			if (symlink_len == -EINVAL) { /* not a symlink or directory */
+				if (is_leaf == 0) {
+					return SET_ERRNO(-ENOTDIR);
+				}
+				else {
+					continue;
+				}
+			}
+			else if ((symlink_len == -ENOENT) && (is_leaf_after_slashes != 0) && (allow_missing_leaf != 0)) { /* non-existing leaf */
 				break;
 			}
 			else {
-				/* errno set by _readlink_abs */
-				return -1;
+				return SET_ERRNO(symlink_len);
 			}
 		}
 
+		if ((is_leaf != 0) && (resolve_last_symlink == 0)) {
+			break;
+		}
+
 		if (symlink_len == 0) {
 			/* NOTE: this case is not defined in POSIX */
 			/* see: https://lwn.net/Articles/551224/ */
@@ -247,8 +254,11 @@ static int _resolve_abspath(char *path, char *result, int resolve_last_symlink,
 		}
 
 		/* prepend resolved symlink to remaining unresolved path */
-		p -= 1;
-		*p = '/';
+		if (*p != '\0') {
+			p -= 1;
+			*p = '/';
+		}
+
 		p -= symlink_len;
 		memmove(p, path, symlink_len);
 
@@ -475,21 +485,27 @@ static ssize_t _readlink_abs(const char *path, char *buf, size_t bufsiz)
 
 	assert(path && path[0] == '/');
 
-	if ((ret = safe_lookup(path, &oid, NULL)) < 0)
-		return SET_ERRNO(ret);
+	ret = safe_lookup(path, &oid, NULL);
+	if (ret < 0) {
+		return ret;
+	}
 
 	msg.type = mtGetAttr;
 	memcpy(&msg.i.attr.oid, &oid, sizeof(oid_t));
 	msg.i.attr.type = atMode;
 
-	if ((ret = msgSend(oid.port, &msg)) != EOK)
-		return SET_ERRNO(ret);
+	ret = msgSend(oid.port, &msg);
+	if (ret != EOK) {
+		return ret;
+	}
 
-	if (msg.o.attr.err != EOK)
-		return SET_ERRNO(msg.o.attr.err);
+	if (msg.o.attr.err != EOK) {
+		return msg.o.attr.err;
+	}
 
-	if (!S_ISLNK(msg.o.attr.val))
-		return SET_ERRNO(-EINVAL);
+	if (!S_ISLNK(msg.o.attr.val)) {
+		return S_ISDIR(msg.o.attr.val) ? -EISDIR : -EINVAL;
+	}
 
 	memset(&msg, 0, sizeof(msg_t));
 	msg.type = mtRead;
@@ -499,13 +515,12 @@ static ssize_t _readlink_abs(const char *path, char *buf, size_t bufsiz)
 	msg.o.size = bufsiz;
 	msg.o.data = buf;
 
-	if ((ret = msgSend(oid.port, &msg)) != EOK)
-		return SET_ERRNO(ret);
-
-	if (msg.o.io.err < 0)
-		return SET_ERRNO(msg.o.io.err);
+	ret = msgSend(oid.port, &msg);
+	if (ret != EOK) {
+		return ret;
+	}
 
-	/* number of bytes written without terminating NULL byte */
+	/* number of bytes written without terminating NULL byte or retcode if < 0 */
 	return msg.o.io.err;
 }
 
@@ -522,8 +537,14 @@ ssize_t readlink(const char *path, char *buf, size_t bufsiz)
 	ret = _readlink_abs(canonical, buf, bufsiz);
 
 	free(canonical);
+	if (ret < 0) {
+		if (ret == -EISDIR) {
+			ret = -EINVAL;
+		}
+
+		return SET_ERRNO(ret);
+	}
 
-	/* if error - errno set by _readlink_abs() */
 	return ret;
 }