From 3f164486df948220fc39f2471c9211086e0ac850 Mon Sep 17 00:00:00 2001 From: Kaz Kylheku Date: Sat, 23 Jul 2022 19:36:52 -0700 Subject: Fix some inaccurate comments. * safepath.c (tamper_proof, safepath_check): Reword outdated comments. --- safepath.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/safepath.c b/safepath.c index bc3246f..19ecf45 100644 --- a/safepath.c +++ b/safepath.c @@ -97,12 +97,11 @@ static int tamper_proof(const struct stat *st) return 0; /* Ownership is good, but permissions are open; object is writable to - * group owner or others. Group writability could be safe, but it's - * complicated to check; we just reject it for clarity and simplicity. + * group owner or others. That could still be safe. */ if ((st->st_mode & (S_IWGRP | S_IWOTH)) != 0) { - /* OK, permissions are open. But is this a directory owned by - * root, which has the sticky bit, such as /tmp? That's OK. + /* Is this a directory owned by root, which has the sticky bit, such + * as /tmp? That's OK. */ if (S_ISDIR(st->st_mode) && (st->st_mode & S_ISVTX) != 0) return 1; @@ -231,8 +230,9 @@ int safepath_check(const char *name) * previous component, which is the directory it lives in. However, * we trust only that link, and not what it points to. It could * point to another link which is not secured against tampering. - * Thus, we must implement symlink resolution right here ourselves, - * applying our rules to every step. Recursion helps. + * Thus, we must implement symlink resolution right here ourselves: + * replace the symlink component with its expansion and continue + * checking the expansion, component by component. */ if (S_ISLNK(st.st_mode)) { char link[256]; @@ -255,11 +255,10 @@ int safepath_check(const char *name) * Either way it's string grafting. */ if (link[0] == '/') { - /* If savechar is zero, we are working with the last - * component. If the last component is an absolute - * symlink, we just recurse on that symlink target. - * Otherwise, we must graft the remainder of the - * path onto the symlink target. + /* If savechar is zero, we are working with the last component. + * If the last component is an absolute symlink, we just replace + * the path with the target, and iterate. Otherwise, we must + * graft the remainder of the path onto the symlink target. */ if (savechar == 0) { free(copy); -- cgit v1.2.3