On 07/03/18 23:17, Stuart Henderson wrote:
> On 2018/06/28 08:52, Martijn van Duren wrote:
>> - If realpath fails the content of resolved is undefined, so why waste
>> cycles setting it to ".". If anyone thinks we should return something
>> sensible I reckon we should make it an empty string.
>
> Not sure if it's important or not, but if someone ignores the failure
> and uses the result anyway (for example perhaps appending "/something"
> to the string), "." seems less likely to cause problems than an empty
> string would.
With my earlier diff it would return the same garbage the user would
have put in, I only mentioned an empty string as a (shorter)
alternative, since we wouldn't have to call strlcpy.
A similar issue is that we don't return a sensible return value in all
the other error cases. resolved is filled up up to and including the
first component that can't be found. If that were to be used with
dirname (e.g. to save a new config in the same dir as the original) it
could end up a directory higher, potentially overwriting something
important.
To see what some of our neighbours do I did a quick test with the
following programs on a stock alpine and debian (only other OSes I
had available):
$ cat test.c
#include <err.h>
#include <limits.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/stat.h>
int
main(int argc, char *argv[])
{
char *path;
path = calloc(PATH_MAX, 1);
path[0] = 'q';
if (mkdir("/tmp/tmp", 0777) == -1)
err(1, "mkdir");
if (chdir("/tmp/tmp") == -1)
err(1, "chdir");
if (rmdir("/tmp/tmp") == -1)
err(1, "rmdir");
if (realpath("test", path) != NULL)
err(1, "realpath");
printf("%s\n", path);
}
alpine$ cc test.c && ./a.out
q
debian$ cc test.c && ./a.out
openbsd$ cc test.c && ./a.out
.
$ cat test1.c
#include <limits.h>
#include <stdlib.h>
#include <stdio.h>
int
main(int argc, char *argv[])
{
char *path;
path = calloc(PATH_MAX, 1);
path[0] = 'q';
printf("%p\n", realpath("/etc/unexistend/whatever.conf", path));
printf("%s\n", path);
}
alpine$ cc test1.c && ./a.out
0
q
debian$ cc test1.c && ./a.out
(nil)
/etc/unexistend
openbsd$ cc test1.c && ./a.out
0x0
/etc/unexistend
So alpine (musl-libc) doesn't touch resolved until it has fully resolved
the path and debian (glibc) clears resolved from the start and fills it
up as it resolves.
Since the behaviour seems to be pretty much all over the place I don't
think it matters much what we do; ergo, just leave resolved as is and
don't change it at all on error. But if we do think people could shoot
themselves in the foot with this we should fix this for a half-filled
resolved as well.
Diff below adds such a possible solution, where risk of misuse is
minimal. It creates the maximum amount of intermediate directories with
the least amount of probability that they exist and leaves no room
at the end (PATH_MAX) to create a file. The only two downside I see are:
1) if some application does hit this *and* decides to and is allowed to
create each and every directory it would be less fun for the admin to
remember how to do: "rm -rf /$(printf '\777')".
2) Execing an application calls realpath multiple times, potentially
failing a couple of times, so the exec-times increase slightly.
>
> One area where this might possibly be triggered is if the user doesn't
> have permissions on the current directory (sometimes I've had problems
> in this area where rc scripts leave cwd as /root but it's mode 700 and
> a uid change has rendered it unreadable by that uid).
>
> If you want to look at users in ports to see what sort of checks they
> do, here's a starter ..
>
> $ grep -lwR realpath ~sthen/ports-nm/ | cut -d/ -f6- | sort
> ...
Index: ./stdlib/realpath.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/realpath.c,v
retrieving revision 1.22
diff -u -p -r1.22 realpath.c
--- ./stdlib/realpath.c 24 Dec 2017 01:50:50 -0000 1.22
+++ ./stdlib/realpath.c 4 Jul 2018 06:08:32 -0000
@@ -49,7 +49,7 @@ realpath(const char *path, char *resolve
char *q;
size_t left_len, resolved_len, next_token_len;
unsigned symlinks;
- int serrno, mem_allocated;
+ int serrno, mem_allocated, i;
ssize_t slen;
char left[PATH_MAX], next_token[PATH_MAX], symlink[PATH_MAX];
@@ -82,13 +82,8 @@ realpath(const char *path, char *resolve
resolved_len = 1;
left_len = strlcpy(left, path + 1, sizeof(left));
} else {
- if (getcwd(resolved, PATH_MAX) == NULL) {
- if (mem_allocated)
- free(resolved);
- else
- strlcpy(resolved, ".", PATH_MAX);
- return (NULL);
- }
+ if (getcwd(resolved, PATH_MAX) == NULL)
+ goto err;
resolved_len = strlen(resolved);
left_len = strlcpy(left, path, sizeof(left));
}
@@ -229,5 +224,12 @@ realpath(const char *path, char *resolve
err:
if (mem_allocated)
free(resolved);
+ else {
+ for (i = 0; i < PATH_MAX - 2;) {
+ resolved[i++] = '/';
+ resolved[i++] = -1;
+ }
+ resolved[i] = '\0';
+ }
return (NULL);
}
