diff options
author | Andreas Gruenbacher <agruen@gnu.org> | 2015-01-31 03:15:08 +0100 |
---|---|---|
committer | Andreas Gruenbacher <agruen@gnu.org> | 2015-01-31 22:14:00 +0100 |
commit | 71a3172c7ecb1fad7965843ba373e99a034ee1ce (patch) | |
tree | ee0738a7a26d749adb0425bdc5e489289fc2c11d | |
parent | 025a54b789bd88ed15430f8633514e296826983e (diff) | |
download | patch-71a3172c7ecb1fad7965843ba373e99a034ee1ce.tar.gz |
Use symlink-safe system call replacements
Use the symlink-safe replacements for system calls in many places throughout
the code: In some places this makes patch safe against path traversal attacks;
in other places, it saves the kernel from having to re-traverse the pathnames.
* src/inp.c (plan_b): Use safe_open() + fdopen() instead of fopen().
* src/util.c (copy_attr): Document why we are safe here.
(create_backup): Use safe_open() instead of creat().
-rw-r--r-- | src/inp.c | 9 | ||||
-rw-r--r-- | src/patch.c | 9 | ||||
-rw-r--r-- | src/pch.c | 3 | ||||
-rw-r--r-- | src/util.c | 75 |
4 files changed, 57 insertions, 39 deletions
@@ -25,6 +25,7 @@ #undef XTERN #define XTERN #include <inp.h> +#include <safe.h> /* Input-file-with-indexable-lines abstract type */ @@ -237,7 +238,7 @@ plan_a (char const *filename) { if (S_ISREG (instat.st_mode)) { - int ifd = open (filename, O_RDONLY|binary_transput); + int ifd = safe_open (filename, O_RDONLY|binary_transput, 0); size_t buffered = 0, n; if (ifd < 0) pfatal ("can't open file %s", quotearg (filename)); @@ -268,7 +269,7 @@ plan_a (char const *filename) else if (S_ISLNK (instat.st_mode)) { ssize_t n; - n = readlink (filename, buffer, size); + n = safe_readlink (filename, buffer, size); if (n < 0) pfatal ("can't read %s %s", "symbolic link", quotearg (filename)); size = n; @@ -339,6 +340,7 @@ plan_a (char const *filename) static void plan_b (char const *filename) { + int ifd; FILE *ifp; int c; size_t len; @@ -351,7 +353,8 @@ plan_b (char const *filename) if (instat.st_size == 0) filename = NULL_DEVICE; - if (! (ifp = fopen (filename, binary_transput ? "rb" : "r"))) + if ((ifd = safe_open (filename, O_RDONLY | binary_transput, 0)) < 0 + || ! (ifp = fdopen (ifd, binary_transput ? "rb" : "r"))) pfatal ("Can't open file %s", quotearg (filename)); if (TMPINNAME_needs_removal) { diff --git a/src/patch.c b/src/patch.c index 7dafb54..64f4391 100644 --- a/src/patch.c +++ b/src/patch.c @@ -34,6 +34,7 @@ #include <gl_linked_list.h> #include <gl_xlist.h> #include <minmax.h> +#include <safe.h> /* procedures */ @@ -291,7 +292,7 @@ main (int argc, char **argv) if (read_only_behavior != RO_IGNORE && ! inerrno && ! S_ISLNK (instat.st_mode) - && access (inname, W_OK) != 0) + && safe_access (inname, W_OK) != 0) { say ("File %s is read-only; ", quotearg (inname)); if (read_only_behavior == RO_WARN) @@ -1919,7 +1920,7 @@ output_files (struct stat const *st) from_st, file_to_output->to, file_to_output->mode, file_to_output->backup); if (file_to_output->to && from_needs_removal) - unlink (file_to_output->from); + safe_unlink (file_to_output->from); if (st && st->st_dev == from_st->st_dev && st->st_ino == from_st->st_ino) { @@ -1949,7 +1950,7 @@ forget_output_files (void) { const struct file_to_output *file_to_output = elt; - unlink (file_to_output->from); + safe_unlink (file_to_output->from); } gl_list_iterator_free (&iter); gl_list_clear (files_to_output); @@ -1973,7 +1974,7 @@ remove_if_needed (char const *name, bool *needs_removal) { if (*needs_removal) { - unlink (name); + safe_unlink (name); *needs_removal = false; } } @@ -32,6 +32,7 @@ #if HAVE_SETMODE_DOS # include <io.h> #endif +#include <safe.h> #define INITHUNKMAX 125 /* initial dynamic allocation size */ @@ -1012,7 +1013,7 @@ prefix_components (char *filename, bool checkdirs) if (checkdirs) { *f = '\0'; - stat_result = stat (filename, &stat_buf); + stat_result = safe_stat (filename, &stat_buf); *f = '/'; if (! (stat_result == 0 && S_ISDIR (stat_buf.st_mode))) break; @@ -52,6 +52,8 @@ # include "verror.h" #endif +#include <safe.h> + static void makedirs (char const *); typedef struct @@ -216,6 +218,9 @@ copy_attr (char const *src_path, char const *dst_path) .quote = copy_attr_quote, .quote_free = copy_attr_free }; + /* FIXME: We are copying between files we know we can safely access by + * pathname. A safe_ version of attr_copy_file() might still be slightly + * more efficient for deep paths. */ return attr_copy_file (src_path, dst_path, copy_attr_check, &ctx); } @@ -244,7 +249,7 @@ set_file_attributes (char const *to, enum file_attributes attr, times[0] = get_stat_atime (st); times[1] = get_stat_mtime (st); } - if (lutimens (to, times) != 0) + if (safe_lutimens (to, times) != 0) pfatal ("Failed to set the timestamps of %s %s", S_ISLNK (mode) ? "symbolic link" : "file", quotearg (to)); @@ -267,10 +272,10 @@ set_file_attributes (char const *to, enum file_attributes attr, /* May fail if we are not privileged to set the file owner, or we are not in group instat.st_gid. Ignore those errors. */ if ((uid != -1 || gid != -1) - && lchown (to, uid, gid) != 0 + && safe_lchown (to, uid, gid) != 0 && (errno != EPERM || (uid != -1 - && lchown (to, (uid = -1), gid) != 0 + && safe_lchown (to, (uid = -1), gid) != 0 && errno != EPERM))) pfatal ("Failed to set the %s of %s %s", (uid == -1) ? "owner" : "owning group", @@ -289,7 +294,7 @@ set_file_attributes (char const *to, enum file_attributes attr, systems where we could. */ if (lchmod (to, mode)) #else - if (! S_ISLNK (mode) && chmod (to, mode) != 0) + if (! S_ISLNK (mode) && safe_chmod (to, mode) != 0) #endif pfatal ("Failed to set the permissions of %s %s", S_ISLNK (mode) ? "symbolic link" : "file", @@ -382,8 +387,8 @@ create_backup (char const *to, const struct stat *to_st, bool leave_original) say ("Creating empty file %s\n", quotearg (bakname)); try_makedirs_errno = ENOENT; - unlink (bakname); - while ((fd = creat (bakname, 0666)) < 0) + safe_unlink (bakname); + while ((fd = safe_open (bakname, O_CREAT | O_WRONLY | O_TRUNC, 0666)) < 0) { if (errno != try_makedirs_errno) pfatal ("Can't create file %s", quotearg (bakname)); @@ -400,7 +405,7 @@ create_backup (char const *to, const struct stat *to_st, bool leave_original) if (debug & 4) say ("Renaming file %s to %s\n", quotearg_n (0, to), quotearg_n (1, bakname)); - while (rename (to, bakname) != 0) + while (safe_rename (to, bakname) != 0) { if (errno == try_makedirs_errno) { @@ -411,7 +416,7 @@ create_backup (char const *to, const struct stat *to_st, bool leave_original) { create_backup_copy (to, bakname, to_st, try_makedirs_errno == 0); - unlink (to); + safe_unlink (to); break; } else @@ -475,7 +480,7 @@ move_file (char const *from, bool *from_needs_removal, char *buffer = xmalloc (PATH_MAX); int fd, size = 0, i; - if ((fd = open (from, O_RDONLY | O_BINARY)) < 0) + if ((fd = safe_open (from, O_RDONLY | O_BINARY, 0)) < 0) pfatal ("Can't reopen file %s", quotearg (from)); while ((i = read (fd, buffer + size, PATH_MAX - size)) > 0) size += i; @@ -496,18 +501,18 @@ move_file (char const *from, bool *from_needs_removal, if (! backup) { - if (unlink (to) == 0) + if (safe_unlink (to) == 0) to_dir_known_to_exist = true; } - if (symlink (buffer, to) != 0) + if (safe_symlink (buffer, to) != 0) { if (errno == ENOENT && ! to_dir_known_to_exist) makedirs (to); - if (symlink (buffer, to) != 0) + if (safe_symlink (buffer, to) != 0) pfatal ("Can't create %s %s", "symbolic link", to); } free (buffer); - if (lstat (to, &to_st) != 0) + if (safe_lstat (to, &to_st) != 0) pfatal ("Can't get file attributes of %s %s", "symbolic link", to); insert_file_id (&to_st, CREATED); } @@ -517,7 +522,7 @@ move_file (char const *from, bool *from_needs_removal, say ("Renaming file %s to %s\n", quotearg_n (0, from), quotearg_n (1, to)); - if (rename (from, to) != 0) + if (safe_rename (from, to) != 0) { bool to_dir_known_to_exist = false; @@ -526,7 +531,7 @@ move_file (char const *from, bool *from_needs_removal, { makedirs (to); to_dir_known_to_exist = true; - if (rename (from, to) == 0) + if (safe_rename (from, to) == 0) goto rename_succeeded; } @@ -535,7 +540,7 @@ move_file (char const *from, bool *from_needs_removal, struct stat tost; if (! backup) { - if (unlink (to) == 0) + if (safe_unlink (to) == 0) to_dir_known_to_exist = true; else if (errno != ENOENT) pfatal ("Can't remove file %s", quotearg (to)); @@ -564,7 +569,7 @@ move_file (char const *from, bool *from_needs_removal, { if (debug & 4) say ("Removing file %s\n", quotearg (to)); - if (unlink (to) != 0 && errno != ENOENT) + if (safe_unlink (to) != 0 && errno != ENOENT) pfatal ("Can't remove file %s", quotearg (to)); } } @@ -583,8 +588,8 @@ create_file (char const *file, int open_flags, mode_t mode, do { if (! (O_CREAT && O_TRUNC)) - close (creat (file, mode)); - fd = open (file, O_CREAT | O_TRUNC | open_flags, mode); + close (safe_open (file, O_CREAT | O_WRONLY | O_TRUNC, mode)); + fd = safe_open (file, O_CREAT | O_TRUNC | open_flags, mode); if (fd < 0) { char *f; @@ -605,7 +610,7 @@ copy_to_fd (const char *from, int tofd) int fromfd; ssize_t i; - if ((fromfd = open (from, O_RDONLY | O_BINARY)) < 0) + if ((fromfd = safe_open (from, O_RDONLY | O_BINARY, 0)) < 0) pfatal ("Can't reopen file %s", quotearg (from)); while ((i = read (fromfd, buf, bufsize)) != 0) { @@ -635,11 +640,11 @@ copy_file (char const *from, char const *to, struct stat *tost, { char *buffer = xmalloc (PATH_MAX); - if (readlink (from, buffer, PATH_MAX) < 0) + if (safe_readlink (from, buffer, PATH_MAX) < 0) pfatal ("Can't read %s %s", "symbolic link", from); - if (symlink (buffer, to) != 0) + if (safe_symlink (buffer, to) != 0) pfatal ("Can't create %s %s", "symbolic link", to); - if (tost && lstat (to, tost) != 0) + if (tost && safe_lstat (to, tost) != 0) pfatal ("Can't get file attributes of %s %s", "symbolic link", to); free (buffer); } @@ -663,7 +668,7 @@ append_to_file (char const *from, char const *to) { int tofd; - if ((tofd = open (to, O_WRONLY | O_BINARY | O_APPEND)) < 0) + if ((tofd = safe_open (to, O_WRONLY | O_BINARY | O_APPEND, 0)) < 0) pfatal ("Can't reopen file %s", quotearg (to)); copy_to_fd (from, tofd); if (close (tofd) != 0) @@ -730,8 +735,8 @@ version_controller (char const *filename, bool readonly, sprintf (trybuf, "%s/", dir); -#define try1(f,a1) (sprintf (trybuf + dirlen, f, a1), stat (trybuf, &cstat) == 0) -#define try2(f,a1,a2) (sprintf (trybuf + dirlen, f, a1,a2), stat (trybuf, &cstat) == 0) +#define try1(f,a1) (sprintf (trybuf + dirlen, f, a1), safe_stat (trybuf, &cstat) == 0) +#define try2(f,a1,a2) (sprintf (trybuf + dirlen, f, a1,a2), safe_stat (trybuf, &cstat) == 0) /* Check that RCS file is not working file. Some hosts don't report file name length errors. */ @@ -862,7 +867,7 @@ version_get (char const *filename, char const *cs, bool exists, bool readonly, cs, readonly ? "" : " with lock"); if (systemic (getbuf) != 0) fatal ("Can't get file %s from %s", quotearg (filename), cs); - if (stat (filename, filestat) != 0) + if (safe_stat (filename, filestat) != 0) pfatal ("%s", quotearg (filename)); } @@ -1301,6 +1306,9 @@ makedirs (char const *name) char *f; char *flim = replace_slashes (filename); + /* FIXME: Now with the pathname lookup cache, there is no reason for + deferring the creation of directories. Callers should be updated. */ + if (flim) { /* Create any missing directories, replacing NULs by '/'s. @@ -1311,7 +1319,7 @@ makedirs (char const *name) for (f = filename; f <= flim; f++) if (!*f) { - mkdir (filename, + safe_mkdir (filename, S_IRUSR|S_IWUSR|S_IXUSR |S_IRGRP|S_IWGRP|S_IXGRP |S_IROTH|S_IWOTH|S_IXOTH); @@ -1341,7 +1349,7 @@ removedirs (char const *name) || ISSLASH (filename[i - 3]))))))) { filename[i] = '\0'; - if (rmdir (filename) == 0 && verbosity == VERBOSE) + if (safe_rmdir (filename) == 0 && verbosity == VERBOSE) say ("Removed empty directory %s\n", quotearg (filename)); filename[i] = '/'; } @@ -1656,10 +1664,15 @@ make_tempfile (char const **name, char letter, char const *real_name, { int fd; + /* gen_tempname(..., GT_NOCREATE) calls lstat() to check if a file + already exists. In the worst case, this leads to a template that + follows a symbolic link and that we cannot use; safe_open() will + detect that. */ + if (gen_tempname (template, 0, flags, GT_NOCREATE)) pfatal ("Can't create temporary file %s", template); retry: - fd = open (template, O_CREAT | O_EXCL | flags, mode); + fd = safe_open (template, O_CREAT | O_EXCL | flags, mode); if (fd == -1) { if (errno == try_makedirs_errno) @@ -1682,7 +1695,7 @@ make_tempfile (char const **name, char letter, char const *real_name, int stat_file (char const *filename, struct stat *st) { int (*xstat)(char const *, struct stat *) = - follow_symlinks ? stat : lstat; + follow_symlinks ? safe_stat : safe_lstat; return xstat (filename, st) == 0 ? 0 : errno; } |