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().
This commit is contained in:
Andreas Gruenbacher 2015-01-31 03:15:08 +01:00
parent 025a54b789
commit 71a3172c7e
4 changed files with 57 additions and 39 deletions

View File

@ -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)
{

View File

@ -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;
}
}

View File

@ -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;

View File

@ -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;
}