[hackers] [sbase] Audit cp() in libutil || FRIGN
commit d823ce2a0fce46428fcbffec6f93eca93479ac15
Author: FRIGN <dev_AT_frign.de>
Date: Thu Mar 19 17:45:30 2015 +0100
Audit cp() in libutil
1) Rename cp_HLPflag -> cp_follow for consistency.
2) Use function-pointers for stat to clear up the code.
3) BUGFIX: TERMINATE THE RESULT BUFFER OF READLINK !!!
It's something I noticed earlier and it actually lead to some
pretty insane behaviour on our side using glibc (musl somehow
magically solves this).
Basically, symlinks used to contain the data of the file they
pointed to. I wondered for weeks where this came from and now
this has finally been solved.
4) BUGFIX: Do not unconditionally unlink target-files. Even GNU
coreutils do it wrong.
The basic idea is this:
If fflag == 0 --> don't touch target files if they exist.
If fflag == 1 --> unlink all and don't error out when we try
to unlink a file which doesn't exist.
5) Use estrlcpy and estrlcat instead of snprintf for path building.
6) Make it clearer what happens in preserve.
diff --git a/cp.c b/cp.c
index 4bba1b5..ebc3566 100644
--- a/cp.c
+++ b/cp.c
_AT_@ -17,7 +17,7 @@ main(int argc, char *argv[])
ARGBEGIN {
case 'a':
- cp_HLPflag = 'P';
+ cp_follow = 'P';
cp_aflag = cp_pflag = cp_rflag = 1;
break;
case 'f':
_AT_@ -36,7 +36,7 @@ main(int argc, char *argv[])
case 'H':
case 'L':
case 'P':
- cp_HLPflag = ARGC();
+ cp_follow = ARGC();
break;
default:
usage();
diff --git a/fs.h b/fs.h
index 91b130f..e7590f8 100644
--- a/fs.h
+++ b/fs.h
_AT_@ -25,7 +25,7 @@ extern int cp_fflag;
extern int cp_pflag;
extern int cp_rflag;
extern int cp_vflag;
-extern int cp_HLPflag;
+extern int cp_follow;
extern int cp_status;
extern int rm_fflag;
diff --git a/libutil/cp.c b/libutil/cp.c
index c6ff48e..32e0dc6 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
_AT_@ -15,136 +15,162 @@
#include "../text.h"
#include "../util.h"
-int cp_aflag = 0;
-int cp_fflag = 0;
-int cp_pflag = 0;
-int cp_rflag = 0;
-int cp_vflag = 0;
+int cp_aflag = 0;
+int cp_fflag = 0;
+int cp_pflag = 0;
+int cp_rflag = 0;
+int cp_vflag = 0;
int cp_status = 0;
-int cp_HLPflag = 'L';
+int cp_follow = 'L';
int
cp(const char *s1, const char *s2, int depth)
{
+ DIR *dp;
FILE *f1, *f2;
- char ns1[PATH_MAX], ns2[PATH_MAX];
struct dirent *d;
struct stat st;
struct utimbuf ut;
- char buf[PATH_MAX];
- DIR *dp;
- int r;
+ ssize_t r;
+ int (*statf)(const char *, struct stat *);
+ char target[PATH_MAX], ns1[PATH_MAX], ns2[PATH_MAX], *statf_name;
- if (cp_vflag)
- printf("'%s' -> '%s'\n", s1, s2);
+ if (cp_follow == 'P' || (cp_follow == 'H' && depth)) {
+ statf_name = "lstat";
+ statf = lstat;
+ } else {
+ statf_name = "stat";
+ statf = stat;
+ }
- r = (cp_HLPflag == 'P' || (cp_HLPflag == 'H' && depth > 0)) ?
- lstat(s1, &st) : stat(s1, &st);
- if (r < 0) {
- weprintf("%s %s:", (cp_HLPflag == 'P' ||
- (cp_HLPflag == 'H' && depth > 0)) ? "lstat" : "stat", s1);
+ if (statf(s1, &st) < 0) {
+ weprintf("%s %s:", statf_name, s1);
cp_status = 1;
return 0;
}
+ if (cp_vflag)
+ printf("%s -> %s\n", s1, s2);
+
if (S_ISLNK(st.st_mode)) {
- if (readlink(s1, buf, sizeof(buf) - 1) >= 0) {
- if (cp_fflag)
- unlink(s2);
- if (symlink(buf, s2) != 0) {
- weprintf("%s: can't create '%s'\n", argv0, s2);
+ if ((r = readlink(s1, target, sizeof(target) - 1)) >= 0) {
+ target[r] = '\0';
+ if (cp_fflag && unlink(s2) < 0 && errno != ENOENT) {
+ weprintf("unlink %s:", s2);
+ cp_status = 1;
+ return 0;
+ } else if (symlink(target, s2) < 0) {
+ weprintf("symlink %s -> %s:", s2, target);
cp_status = 1;
return 0;
}
}
- goto preserve;
- }
+ } else if (S_ISDIR(st.st_mode)) {
+ if (!cp_rflag) {
+ weprintf("%s is a directory\n", s1);
+ cp_status = 1;
+ return 0;
+ }
+ if (!(dp = opendir(s1))) {
+ weprintf("opendir %s:", s1);
+ cp_status = 1;
+ return 0;
+ }
+ if (mkdir(s2, st.st_mode) < 0 && errno != EEXIST) {
+ weprintf("mkdir %s:", s2);
+ cp_status = 1;
+ return 0;
+ }
- if (S_ISDIR(st.st_mode)) {
- if (!cp_rflag)
- eprintf("%s: is a directory\n", s1);
+ while ((d = readdir(dp))) {
+ if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
+ continue;
- if (!(dp = opendir(s1)))
- eprintf("opendir %s:", s1);
+ estrlcpy(ns1, s1, sizeof(ns1));
+ if(s1[strlen(s1) - 1] != '/')
+ estrlcat(ns1, "/", sizeof(ns1));
+ estrlcat(ns1, d->d_name, sizeof(ns1));
- if (mkdir(s2, st.st_mode) < 0 && errno != EEXIST)
- eprintf("mkdir %s:", s2);
+ estrlcpy(ns2, s2, sizeof(ns2));
+ if(s2[strlen(s2) - 1] != '/')
+ estrlcat(ns2, "/", sizeof(ns2));
+ estrlcat(ns2, d->d_name, sizeof(ns2));
- while ((d = readdir(dp))) {
- if (strcmp(d->d_name, ".") && strcmp(d->d_name, "..")) {
- r = snprintf(ns1, sizeof(ns1), "%s/%s", s1, d->d_name);
- if (r >= sizeof(ns1) || r < 0) {
- eprintf("%s/%s: filename too long\n",
- s1, d->d_name);
- }
- r = snprintf(ns2, sizeof(ns2), "%s/%s", s2, d->d_name);
- if (r >= sizeof(ns2) || r < 0) {
- eprintf("%s/%s: filename too long\n",
- s2, d->d_name);
- }
- fnck(ns1, ns2, cp, depth + 1);
- }
+ fnck(ns1, ns2, cp, depth + 1);
}
- closedir(dp);
- goto preserve;
- }
- if (cp_aflag) {
- if (S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode) ||
- S_ISSOCK(st.st_mode) || S_ISFIFO(st.st_mode)) {
- unlink(s2);
- if (mknod(s2, st.st_mode, st.st_rdev) < 0) {
- weprintf("%s: can't create '%s':", argv0, s2);
+ closedir(dp);
+ } else if (cp_aflag && (S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode) ||
+ S_ISSOCK(st.st_mode) || S_ISFIFO(st.st_mode))) {
+ if (cp_fflag && unlink(s2) < 0 && errno != ENOENT) {
+ weprintf("unlink %s:", s2);
+ cp_status = 1;
+ return 0;
+ } else if (mknod(s2, st.st_mode, st.st_rdev) < 0) {
+ weprintf("mknod %s:", s2);
+ cp_status = 1;
+ return 0;
+ }
+ } else {
+ if (!(f1 = fopen(s1, "r"))) {
+ weprintf("fopen %s:", s1);
+ cp_status = 1;
+ return 0;
+ }
+ if (!(f2 = fopen(s2, "w"))) {
+ if (cp_fflag) {
+ if (unlink(s2) < 0 && errno != ENOENT) {
+ weprintf("unlink %s:", s2);
+ cp_status = 1;
+ return 0;
+ } else if (!(f2 = fopen(s2, "w"))) {
+ weprintf("fopen %s:", s2);
+ cp_status = 1;
+ return 0;
+ }
+ } else {
+ weprintf("fopen %s:", s2);
cp_status = 1;
return 0;
}
- goto preserve;
}
- }
+ concat(f1, s1, f2, s2);
- if (!(f1 = fopen(s1, "r"))) {
- weprintf("fopen %s:", s1);
- cp_status = 1;
- return 0;
- }
+ /* preserve permissions by default */
+ fchmod(fileno(f2), st.st_mode);
- if (!(f2 = fopen(s2, "w"))) {
- if (cp_fflag) {
- unlink(s2);
- if (!(f2 = fopen(s2, "w"))) {
- weprintf("fopen %s:", s2);
- cp_status = 1;
- return 0;
- }
- } else {
- weprintf("fopen %s:", s2);
+ if (fclose(f2) == EOF) {
+ weprintf("fclose %s:", s2);
+ cp_status = 1;
+ return 0;
+ }
+ if (fclose(f1) == EOF) {
+ weprintf("fclose %s:", s1);
cp_status = 1;
return 0;
}
}
- concat(f1, s1, f2, s2);
- /* preserve permissions by default */
- fchmod(fileno(f2), st.st_mode);
- fclose(f2);
- fclose(f1);
-preserve:
if (cp_aflag || cp_pflag) {
- if (!(S_ISLNK(st.st_mode))) {
- /* timestamp */
+ /* timestamp and owner*/
+ if (!S_ISLNK(st.st_mode)) {
ut.actime = st.st_atime;
ut.modtime = st.st_mtime;
utime(s2, &ut);
- }
- /* preserve owner ? */
- if (S_ISLNK(st.st_mode))
- r = lchown(s2, st.st_uid, st.st_gid);
- else
- r = chown(s2, st.st_uid, st.st_gid);
- if (r < 0) {
- weprintf("cp: can't preserve ownership of '%s':", s2);
- cp_status = 1;
+
+ if (chown(s2, st.st_uid, st.st_gid) < 0) {
+ weprintf("chown %s:", s2);
+ cp_status = 1;
+ return 0;
+ }
+ } else {
+ if (lchown(s2, st.st_uid, st.st_gid) < 0) {
+ weprintf("lchown %s:", s2);
+ cp_status = 1;
+ return 0;
+ }
}
}
+
return 0;
}
diff --git a/mv.c b/mv.c
index 404a310..9d5b77a 100644
--- a/mv.c
+++ b/mv.c
_AT_@ -18,7 +18,7 @@ mv(const char *s1, const char *s2, int depth)
return (mv_status = 0);
if (errno == EXDEV) {
cp_aflag = cp_rflag = cp_pflag = 1;
- cp_HLPflag = 'P';
+ cp_follow = 'P';
rm_rflag = 1;
cp(s1, s2, depth);
rm(s1, NULL, NULL, &r);
Received on Fri Mar 20 2015 - 21:22:28 CET
This archive was generated by hypermail 2.3.0
: Fri Mar 20 2015 - 21:24:10 CET