From: =?utf-8?q?Christian_G=C3=B6ttsche?= Date: Fri, 24 Jul 2020 17:55:36 +0200 Subject: chkdirs: Simplify, fix compiler issues, spelling mistake, and return value MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit 1. fix return logic Author: Christian Göttsche Date: Fri Jul 24 17:55:36 2020 +0200 If called with multiple arguments, fail if any directory fails, not only the last one. 2. fix spelling error in error message Author: Richard Lewis Date: Fri Oct 15 22:55:03 2021 +0100 Forwarded: yes - (forwarded by email, 12 mar 2023) Replaces 'WARNIING' with 'WARNING' and removes trailing whitespace 3. Various compilation issues Author: Christian Göttsche Date: Fri Jul 24 17:55:36 2020 +0200 Last-Updated: 2021-10-10 Forwarded: https://lists.debian.org/debian-security-tools/2021/10/msg00006.html a) chkdirs.c: In function ‘make_pathname’: chkdirs.c:73:38: error: comparison of integer expressions of different signedness: ‘long unsigned int’ and ‘int’ [-Werror=sign-compare] 73 | if (!(*buffer) || (sizeof(*buffer) < pathname_len)) { | ^ chkdirs.c:182:7: warning: Potential leak of memory pointed to by 'dl' fprintf(stderr, "lstat(%s/%s): %s\n", ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/x86_64-linux-gnu/bits/stdio2.h:113:3: note: expanded from macro 'fprintf' __fprintf_chk (stream, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__) ^~~~~~~~~~~~~ b) chkdirs.c: In function ‘usage’: chkdirs.c:56:6: error: function might be candidate for attribute ‘noreturn’ [-Werror=suggest-attribute=noreturn] 56 | void usage () | ^~~~~ c) chkdirs: use strdup to avoid stringop-overflow warning /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ chkdirs.c: In function ‘check_dir’: chkdirs.c:111:25: note: length computed here 111 | if (!path || !(plen = strlen(path))) { | ^~~~~~~~~~~~ d) chkdirs: fix dead code - Found by Cppcheck In line 72 buffer is forced to be non-NULL, because it got dereferenced in line 71. chkdirs.c:71:10: warning: Either the condition 'if(buffer)' is redundant or there is possible null pointer dereference: buffer. [nullPointerRedundantCheck] if (!(*buffer) || (sizeof(*buffer) < pathname_len)) { ^ chkdirs.c:72:8: note: Assuming that condition 'if(buffer)' is not redundant if (buffer) free((void *)*buffer); ^ chkdirs.c:71:10: note: Null pointer dereference if (!(*buffer) || (sizeof(*buffer) < pathname_len)) { ^ e) chkdirs: fix memory leak - Found by Cppcheck chkdirs.c:126:2: error: Memory leak: curpath [memleak] return(-1); ^ 5. chkdirs: simplify Author: richard.lewis.debian@googlemail.com i) Remove use of linked lists and malloc: previous implementation creates a linked list of things to check, and then immediately checks recursively and frees the list: Just check recursively, and rather than mallocing a buffer, just reuse a single one buffer. ii) Ensure NAME_MAX is always defined. iii) Improve error message if we find an unsupported file system, (it's not just BTRFS but also OVERLAYFS that) Forwarded: yes (Forwarded by email: 21 Dec 2024) Last-Updated: 2024-11-06 --- chkdirs.c | 274 +++++++++++++++++++++++++++++--------------------------------- 1 file changed, 126 insertions(+), 148 deletions(-) diff --git a/chkdirs.c b/chkdirs.c index 0a512f7..a41520c 100644 --- a/chkdirs.c +++ b/chkdirs.c @@ -28,10 +28,10 @@ */ -#if defined(__OpenBSD__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__sun) || defined (hpux) || defined (__bsdi__) || defined (bsdi) || defined (__APPLE__) -#include -#elif defined(__APPLE__) && defined(__MACH__) +#if defined(__APPLE__) || defined(__MACH__) #include +#else +#include #endif #include @@ -44,176 +44,151 @@ #include #ifndef NAME_MAX -#define NAME_MAX PATH_MAX +#ifndef PATH_MAX +#define PATH_MAX 4096 +#endif +#define NAME_MAX PATH_MAX #endif -struct dirinfolist { - char dil_name[NAME_MAX+1]; - int dil_lc; - struct dirinfolist *dil_next; -}; - +#ifdef __linux__ +#include "sys/statfs.h" +#include "linux/magic.h" +const char* file_system_name(__fsword_t f_type) { + // we only need to list problematic types here + switch (f_type) { + case OVERLAYFS_SUPER_MAGIC: + return "overlayfs"; + case BTRFS_TEST_MAGIC: + return "btrfs"; + default: + return "unsupported type"; + } +} +#else +typedef long __fsword_t; +struct statfs { __fsword_t f_type; }; +const char* file_system_name(__fsword_t f_type) { + return "unsupported"; +} +long statfs(const char *path, struct statfs *buf){ + return 1; +} +#endif -void usage () +#ifdef __GNUC__ +__attribute__((__noreturn__)) +#endif +void usage (void) { fprintf(stderr, "chkdirs [-n] dir ...\n"); exit(255); } -char *make_pathname (char *path, char *dir, char **buffer) -{ - int plen, pathname_len, bufsize, offs; - - bufsize = 0; - plen = strlen(path); - pathname_len = plen + strlen(dir) + 2; +// append dir to path (and add a \0). path should start if a string of +// length pathlen (excluding any \0). we return the new length +// (excluding the terminating \0 which we add). This means path must +// be able to grow to NAME_MAX including the dir, a trailing / and a +// new \0 -- If new length will not fit we exit(2). dir should not end +// in a / -- one will be added. +int append_to_path(char* path, int pathlen, const char* dir){ + int dirlen=strlen(dir); + int newpathlen=pathlen+dirlen+1; // old+new+/ - if (!(*buffer) || (sizeof(*buffer) < pathname_len)) { - if (buffer) free((void *)*buffer); - bufsize = (pathname_len > PATH_MAX) ? pathname_len : PATH_MAX; - if (!(*buffer = (char *)malloc(bufsize))) { - return((char *)NULL); - } + if (newpathlen > NAME_MAX-1) { + fprintf(stderr, "chkdirs: ERROR: path too long: %s/%s\n", path, dir); + exit(2); } - if (dir[0] == '/') { /* "dir" is absolute pathname, don't prepend "path" */ - offs = 0; - } - else { - strncpy(*buffer, path, bufsize); - if ((*buffer)[plen-1] == '/') { /* "path" ends in "/", don't add extra */ - offs = plen; - } - else { - (*buffer)[plen] = '/'; - offs = plen + 1; - } - } - strncpy((*buffer)+offs, dir, bufsize - offs); - return((*buffer)); + path[pathlen]='/'; // replace \0 + memcpy(path+pathlen,dir,dirlen); // apppend dir, overwriting \0 + path[newpathlen-1]='/'; // append / + path[newpathlen]='\0'; // nb: newpathlen <= NAME_MAX-1 + + return newpathlen; } -int check_dir (char *dir, char *path, int linkcount, int norecurse) +/* Check path for hidden files. + - path = pointer to buffer holding path of a dir + to check. This will be modified if we recurse + - pathlen = strlen(path) + - linkcount = number of links in path (as seen by + parent). If 0: stat path to set this) + - norecurse = 0 or 1 - should we recurse (1=no) into subdirs + + returns: 0 (all ok) / 1 (hidden files) / 2 (error) + on errors, subsequent calls may fail if path is relative + */ + +int check_dir (char *path, int pathlen, int linkcount, int norecurse) { - int diff = -1; - int plen, buflen, numdirs; - char *curpath, *fullpath; + int diff = -1, retval = 0, numdirs = 0; + DIR *dirhandle; struct dirent *finfo; - struct dirinfolist *dl, *dptr; struct stat statinfo; - /* When called recursively, "path" will be the full path of the cwd, - but when called from main() "path" is empty. We need the cwd path - so we can chdir() back at the end of this routine, as well as when - printing errors and other output. - */ - if (!path || !(plen = strlen(path))) { - buflen = PATH_MAX; - retry: - if (!(curpath = (char *)malloc(buflen))) { - fprintf(stderr, "malloc() failed: %s\n", strerror(errno)); - return(-1); - } - if (!getcwd(curpath, buflen)) { - if (errno == ERANGE) { - free((void *)curpath); - buflen = buflen * 2; - goto retry; - } - else { - fprintf(stderr, "getcwd() failed: %s\n", strerror(errno)); - return(-1); - } - } - } - else { /* "path" is set, so just copy it into "curpath" */ - if (!(curpath = (char *)malloc(plen+1))) { - fprintf(stderr, "malloc() failed: %s\n", strerror(errno)); - return(-1); - } - strncpy(curpath, path, plen+1); - } - - /* Now set "fullpath" to be the absolute path name of the directory - we will be checking (prepend "curpath" if "dir" is not already an - absolute pathname). - */ - fullpath = (char *)NULL; - if (!make_pathname(curpath, dir, &fullpath)) { - fprintf(stderr, "make_pathname() failed: %s\n", strerror(errno)); - free((void *)curpath); - return(-1); - } + int newpathlen; // length of 'path/subdir' - if (chdir(dir)) { - fprintf(stderr, "chdir(%s): %s\n", fullpath, strerror(errno)); - free((void *)curpath); - free((void *)fullpath); - return(-1); + if (chdir(path)) { + fprintf(stderr, "chkdirs: WARNING: chdir(%s): %s\n", path, strerror(errno)); + return(2); } - /* Again, "linkcount" (the link count of the current directory) is set - only if check_dir() is called recursively. Otherwise, we need to + /* "linkcount" (the link count of the current directory) is non-zero + only if check_dir() was called recursively. Otherwise, we need to stat the directory ourselves. */ if (!linkcount) { if (lstat(".", &statinfo)) { - fprintf(stderr, "lstat(%s): %s\n", fullpath, strerror(errno)); - goto abort; + fprintf(stderr, "chkdirs: WARNING: cannot read linkcount: lstat(%s): %s\n", path, strerror(errno)); + return(2); } linkcount = statinfo.st_nlink; - if (linkcount == 1) + if (linkcount == 1) { - fprintf(stderr, "WARNIING: It seems you are using BTRFS, if this is true chkdirs can't help you to find hidden files/dirs\n"); - goto abort; - } + struct statfs fstatinfo; + if (statfs(".",&fstatinfo)) + fprintf(stderr, "chkdirs: WARNING: Skipping unsupported filesystem (could not check type): %s\n", path); + else + fprintf(stderr, "chkdirs: WARNING: Skipping unsupported filesystem (%s): %s\n", file_system_name(fstatinfo.f_type), path); + return(2); + } } if (!(dirhandle = opendir("."))) { - fprintf(stderr, "opendir(%s): %s\n", fullpath, strerror(errno)); - goto abort; + fprintf(stderr, "chkdirs: WARNING: opendir(%s): %s\n", path, strerror(errno)); + return(2); } - numdirs = 0; - dl = (struct dirinfolist *)NULL; while ((finfo = readdir(dirhandle))) { if (!strcmp(finfo->d_name, ".") || !strcmp(finfo->d_name, "..")) continue; if (lstat(finfo->d_name, &statinfo)) { - fprintf(stderr, "lstat(%s/%s): %s\n", - fullpath, finfo->d_name, strerror(errno)); + fprintf(stderr, "chkdirs: WARNING: cannot lstat(%s/%s): %s\n", + path, finfo->d_name, strerror(errno)); closedir(dirhandle); - goto abort; + retval = 2; + continue; } if (S_ISDIR(statinfo.st_mode)) { - numdirs++; + ++numdirs; - if (norecurse) continue; /* just count subdirs if "-n" */ + if (norecurse) + continue; /* just count subdirs if "-n" */ - /* Otherwise, keep a list of all directories found that have link - count > 2 (indicating directory contains subdirectories). We'll - call check_dir() on each of these subdirectories in a moment... - */ - if (statinfo.st_nlink > 2) { - dptr = dl; - if (!(dl = (struct dirinfolist *)malloc(sizeof(struct dirinfolist)))) { - fprintf(stderr, "malloc() failed: %s\n", strerror(errno)); - norecurse = 1; - while (dptr) { - dl = dptr->dil_next; - free((void *)dptr); - dptr = dl; - } - continue; - } + if (statinfo.st_nlink > 2) { // why trust st.nlink here ????? + newpathlen = append_to_path(path, pathlen, finfo->d_name); + diff = check_dir(path, newpathlen, statinfo.st_nlink, norecurse); + if (diff > retval) retval = diff; + path[pathlen] = '\0'; // reset path - strncpy(dl->dil_name, finfo->d_name, sizeof(dl->dil_name)); - dl->dil_lc = statinfo.st_nlink; - dl->dil_next = dptr; + if (chdir(path)) { // reset cwd + fprintf(stderr, "chkdirs: WARNING: Final chdir(%s) failed (%s) -- EXIT!\n", path, strerror(errno)); + return(2); + } } } } @@ -221,33 +196,21 @@ int check_dir (char *dir, char *path, int linkcount, int norecurse) /* Parent directory link count had better equal #subdirs+2... */ diff = linkcount - numdirs - 2; - if (diff) printf("%d\t%s\n", diff, fullpath); - - /* Now check all subdirectories in turn... */ - while (dl) { - check_dir(dl->dil_name, fullpath, dl->dil_lc, norecurse); - dptr = dl->dil_next; - free((void *)dl); - dl = dptr; + if (diff) { + if (retval < 2) retval = 1; + printf("Possible hidden file(s) in %s: expected %d link(s) but found %d\n", path, numdirs, linkcount - 2); } - - abort: - if (chdir(curpath)) { - fprintf(stderr, "Final chdir(%s) failed (%s) -- EXIT!\n", - curpath, strerror(errno)); - exit(255); - } - free((void *)fullpath); - free((void *)curpath); - return(diff); + return(retval); } int main (int argc, char **argv) { int norecurse = 0; - int i, retval; + int i, retval = 0; int c; + char path[NAME_MAX]=""; + int pathlen=0; opterr = 0; while ((c = getopt(argc, argv, "n")) > 0) { @@ -261,8 +224,23 @@ int main (int argc, char **argv) } if (argc <= optind) usage(); - for (i = optind; i < argc; i++) { - retval = check_dir(argv[i], (char *)NULL, 0, norecurse); + for (i = optind; i < argc; ++i) { + if (argv[i][0]!='/'){ + // relative dir: prepend cwd + if (getcwd(path, NAME_MAX)==NULL) { + fprintf(stderr, "chkdirs: WARNING: getwd(%s): %s\n", argv[i], strerror(errno)); + retval = 2; + continue; + } + pathlen = append_to_path(path, strlen(path), ""); // add trailing / + } else { + pathlen = 0; + path[0] = '\0'; + } + pathlen = append_to_path(path, pathlen, argv[i]); + c = check_dir(path, pathlen, 0, norecurse); + if (c > retval) + retval = c; } exit(retval); }