/etc/passwd and symlinks

[Ralf Friedl]

Paul Fox wrote:
> denys wrote:
>  > 
>  > Thus I propose introducing xmalloc_readlink_recursive()
>  > which does not suffer from those two problems,
>  > and using it here (and elsewhere: insmod, syslogd -
>  > grep for xmalloc_readlink).
>
> well, i've written a new version, but it doesn't do what you
> thought it was going to.  :-)
>
> realpath() fully canonicalizes a path -- it makes it absolute,
> removes "/..", and expands symlinks all along the path.
>
> readlink() (and also xmalloc_readlink()) simply returns the value
> of the named symlink.  (which makes the switch from realpath()
> to xmalloc_readlink() somewhat suspicious, by the way.)
>   
I think that should be no problem. The path will eventually point to a 
real file, and as long as the new temporary file is created in the same 
directory, it should be possible to rename it later.

Also, I don't see why insmod should need the realpath of the object. I 
thought the basename of the object file is used as the module name, but 
I have also seen few cases where the module name was not the file name, 
so that seems to be only a convention and not mandatory.
> i've written xmalloc_readlink_follow(char *path) which iteratively
> does textual expansion of path, expanding symlinks at the tail end
> of path.  no other canonicalization is done, and the result is not
> necessarily absolute.  this is the minimum needed for my "allow
> /etc/passwd to be a symlink" change, and it's probably useful for
> most of the other current uses of xmalloc_readlink().
>   
I noticed that you test against MAXSYMLINKS only in the case of relative 
symlinks. I consider this inconsistent.
In the case of /etc/passwd, the administrator probably can be trusted to 
not create a loop. But suppose you have an absolute symlink /etc/passwd 
-> /etc/passwd. The function would loop forever, while a stat on 
/etc/passwd would fail.with ELOOP. With a relative symlink /etc/passwd 
-> passwd the loop would be detected.

Another question is what the function should do if the target of the 
link does not exist. xmalloc_readlink will always return the target 
name, whether it exists or not. I don't know whether this behavior would 
be useful for xmalloc_readlink_follow or not.

Regards
Ralf Friedl
> /*
>  * this routine is not the same as realpath(), which canonicalizes
>  * the given path completely.  this routine only follows trailing
>  * symlinks until a real file is reached, and returns its name. 
>  * intermediate symlinks are not expanded.  as above, a malloced
>  * char* is returned, which must be freed.
>  */
> char *xmalloc_readlink_follow(const char *path)
> {
> 	char *buf = NULL, *lpc, *linkpath;
> 	int bufsize;
> 	smallint looping = 0;
>
> 	buf = strdup(path);
> 	bufsize = strlen(path) + 1;
>
> 	while(1) {
> 		linkpath = xmalloc_readlink(buf);
> 		if (!linkpath) {
> 			if (errno == EINVAL) /* not a symlink */
> 				return buf;
> 			free(buf);
> 			return NULL;
> 		} 
>
> 		if (*linkpath == '/') {
> 			free(buf);
> 			buf = linkpath;
> 			bufsize = strlen(linkpath) + 1;
> 		} else {
> 			bufsize += strlen(linkpath);
> 			if (looping++ > MAXSYMLINKS) {
> 				free(linkpath);
> 				free(buf);
> 				return NULL;
> 			}
> 			buf = xrealloc(buf, bufsize);
> 			lpc = bb_get_last_path_component_strip(buf);
> 			strcpy(lpc, linkpath);
> 			free(linkpath);
> 		}
> 	}
> }