rsync - Jan 2004 - rsync 2.6.0: possible sanitization bug?

Hiya.

While merging the 2.6.0 changes into our modified version of rsync, I
noticed the following bit of code in 2.6.0's options.c:

        extern int sanitize_paths;
        if (sanitize_paths)
                sanitize_path(strdup(files_from), NULL);
        filesfrom_fd = open(files_from, O_RDONLY|O_BINARY);

Since sanitize_path modifies its first argument in place, the path that
open() gets there hasn't been sanitized, which could be a security issue
-- plus it leaks memory.  Shouldn't that be something like this?

        extern int sanitize_paths;
        char *s = strdup(files_from);
        if (sanitize_paths)
                sanitize_path(s, NULL);
        filesfrom_fd = open(s, O_RDONLY|O_BINARY);
        free(s);

Thanks,

-- 
Adam Sampson <azz@us-lot.org>                       
<http://offog.org/>

On Fri, Jan 30, 2004 at 11:52:17AM +0000, Adam Sampson
wrote:
> Since sanitize_path modifies its first argument in place, the path
> that open() gets there hasn't been sanitized
Ouch!  I've just checked in a fix.  I didn't free the strdup() memory
because I wanted to leave the value sanitized, just in case the code
needs to know the actual value that was opened (in some future change).

Thanks for the report!

..wayne..
-------------- next part --------------
--- options.c	27 Jan 2004 23:13:12 -0000	1.128
+++ options.c	30 Jan 2004 16:13:40 -0000
@@ -686,8 +686,10 @@ int parse_arguments(int *argc, const cha
 			}
 		} else {
 			extern int sanitize_paths;
-			if (sanitize_paths)
-				sanitize_path(strdup(files_from), NULL);
+			if (sanitize_paths) {
+				files_from = strdup(files_from);
+				sanitize_path(files_from, NULL);
+			}
 			filesfrom_fd = open(files_from, O_RDONLY|O_BINARY);
 			if (filesfrom_fd < 0) {
 				rsyserr(FERROR, errno,