unsafe_symlink change (Re: CVS update: rsync)

[Wayne Davison]

On Thu, Jan 16, 2003 at 02:33:26PM -0600, Dave Dykstra wrote:
> I think you could get rid of the calls to strdup() and the new local
> variables and possibly do a couple casts inside the function instead.

You wouldn't want to carve up the const strings with strtok() (which
adds nulls to the strings).  The function could be rewritten to scan the
string for '/'s without using strtok(), which would be a more efficient
way to go (eliminating the need for the strdups).

Here's a quick rewrite that I minimally tested.  See if you like it.

..wayne..

---8<------8<------8<------8<---cut here--->8------>8------>8------>8---
Index: util.c
--- util.c	15 Jan 2003 17:49:44 -0000	1.118
+++ util.c	17 Jan 2003 01:26:36 -0000
@@ -793,49 +793,38 @@
  *
  * @sa t_unsafe.c
  **/
-int unsafe_symlink(const char *dest_path, const char *src_path)
+int unsafe_symlink(const char *dest, const char *src)
 {
-	char *tok, *src, *dest;
+	const char *name, *slash;
 	int depth = 0;
 
 	/* all absolute and null symlinks are unsafe */
-	if (!dest_path || !*dest_path || *dest_path == '/') return 1;
-
-	src = strdup(src_path);
-	if (!src) out_of_memory("unsafe_symlink");
+	if (!dest || !*dest || *dest == '/') return 1;
 
 	/* find out what our safety margin is */
-	for (tok=strtok(src,"/"); tok; tok=strtok(NULL,"/")) {
-		if (strcmp(tok,"..") == 0) {
+	for (name = src; (slash = strchr(name, '/')); name = slash+1) {
+		if (strncmp(name, "../", 3) == 0) {
 			depth=0;
-		} else if (strcmp(tok,".") == 0) {
+		} else if (strncmp(name, "./", 2) == 0) {
 			/* nothing */
 		} else {
 			depth++;
 		}
 	}
-	free(src);
-
-	/* drop by one to account for the filename portion */
-	depth--;
-
-	dest = strdup(dest_path);
-	if (!dest) out_of_memory("unsafe_symlink");
 
-	for (tok=strtok(dest,"/"); tok; tok=strtok(NULL,"/")) {
-		if (strcmp(tok,"..") == 0) {
-			depth--;
-		} else if (strcmp(tok,".") == 0) {
+	for (name = dest; (slash = strchr(name, '/')); name = slash+1) {
+		if (strncmp(name, "../", 3) == 0) {
+			/* if at any point we go outside the current directory
+			   then stop - it is unsafe */
+			if (--depth < 0)
+				break;
+		} else if (strncmp(name, "./", 2) == 0) {
 			/* nothing */
 		} else {
 			depth++;
 		}
-		/* if at any point we go outside the current directory then
-		   stop - it is unsafe */
-		if (depth < 0) break;
 	}
 
-	free(dest);
 	return (depth < 0);
 }
 
---8<------8<------8<------8<---cut here--->8------>8------>8------>8---