github-repos/wget
1
0
Fork 0
mirror of https://github.com/mirror/wget.git synced 2025-02-05 09:10:12 +08:00
Code Issues Packages Projects Releases Wiki Activity

Safeguards against TOCTTOU

Browse Source 
* src/utils.h: Add struct file_stat_s declaration,
  change prototypes of file_exists_p(),
  add prototypes for fopen_stat() and open_stat().
* src/utils.c: Extend file_exists_p(),
  new function fopen_stat() and open_stat(),
  add new param for file_exists_p().
* src/init.h: Add param file_stats_t to run_wgetrc().
* src/ftp.c: Amend calls to extended functions.
* src/hsts.c: Likewise.
* src/http.c: Likewise.
* src/init.c: Likewise.
* src/main.c: Likewise.
* src/metalink.c: Likewise.
* src/retr.c: Likewise.
* src/url.c: Likewise.

Added fopen_stat() and open_stat() that checks to makes sure the file didn't
change underneath us.
Return error from file_exists_p().
Added a way to return error from this file without major surgery to the
callers.

Fixes: #20369
This commit is contained in:
Vijo Cherian 2017-03-05 21:22:00 -08:00 committed by Tim Rühsen
parent 90a0a7499c
commit 400b8eba6c
11 changed files with 197 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/ftp.c
View File

@ -1463,7 +1463,7 @@ Error in server response, closing control connection.\n"));
else if (opt.noclobber || opt.always_rest || opt.timestamping || opt.dirstruct
|| opt.output_document || count > 0)
{
if (opt.unlink_requested && file_exists_p (con->target))
if (opt.unlink_requested && file_exists_p (con->target, NULL))
{
if (unlink (con->target) < 0)
{
@ -1857,7 +1857,7 @@ ftp_loop_internal (struct url *u, struct url *original_url, struct fileinfo *f,
/* If we receive .listing file it is necessary to determine system type of the ftp
server even if opn.noclobber is given. Thus we must ignore opt.noclobber in
order to establish connection with the server and get system type. */
if (opt.noclobber && !opt.output_document && file_exists_p (con->target)
if (opt.noclobber && !opt.output_document && file_exists_p (con->target, NULL)
&& !((con->cmd & DO_LIST) && !(con->cmd & DO_RETR)))
{
logprintf (LOG_VERBOSE,
@ -2413,7 +2413,7 @@ Already have correct symlink %s -> %s\n\n"),
&& !(f->type == FT_SYMLINK && !opt.retr_symlinks)
&& f->tstamp != -1
&& dlthis
&& file_exists_p (con->target))
&& file_exists_p (con->target, NULL))
{
touch (actual_target, f->tstamp);
}

7
src/hsts.c
View File

@ -32,9 +32,9 @@ as that of the covered work. */
#ifdef HAVE_HSTS
#include "hsts.h"
#include "utils.h"
#include "host.h" /* for is_valid_ip_address() */
#include "init.h" /* for home_dir() */
#include "utils.h"
#include "hash.h"
#include "c-ctype.h"
#ifdef TESTING
@ -500,18 +500,19 @@ hsts_store_t
hsts_store_open (const char *filename)
{
hsts_store_t store = NULL;
file_stats_t fstats;
store = xnew0 (struct hsts_store);
store->table = hash_table_new (0, hsts_hash_func, hsts_cmp_func);
store->last_mtime = 0;
store->changed = false;
if (file_exists_p (filename))
if (file_exists_p (filename, &fstats))
{
if (hsts_file_access_valid (filename))
{
struct stat st;
FILE *fp = fopen (filename, "r");
FILE *fp = fopen_stat (filename, "r", &fstats);
if (!fp || !hsts_read_database (store, fp, false))
{

14
src/http.c
View File

@ -2290,7 +2290,7 @@ check_file_output (const struct url *u, struct http_stat *hs,
}
/* TODO: perform this check only once. */
if (!hs->existence_checked && file_exists_p (hs->local_file))
if (!hs->existence_checked && file_exists_p (hs->local_file, NULL))
{
if (opt.noclobber && !opt.output_document)
{
@ -2486,7 +2486,7 @@ open_output_stream (struct http_stat *hs, int count, FILE **fp)
}
else if (ALLOW_CLOBBER || count > 0)
{
if (opt.unlink_requested && file_exists_p (hs->local_file))
if (opt.unlink_requested && file_exists_p (hs->local_file, NULL))
{
if (unlink (hs->local_file) < 0)
{
@ -4050,7 +4050,7 @@ http_loop (const struct url *u, struct url *original_url, char **newloc,
got_name = true;
}
if (got_name && file_exists_p (hstat.local_file) && opt.noclobber && !opt.output_document)
if (got_name && file_exists_p (hstat.local_file, NULL) && opt.noclobber && !opt.output_document)
{
/* If opt.noclobber is turned on and file already exists, do not
retrieve the file. But if the output_document was given, then this
@ -4087,7 +4087,7 @@ http_loop (const struct url *u, struct url *original_url, char **newloc,
{
/* Use conditional get request if requested
* and if timestamp is known at this moment. */
if (opt.if_modified_since && !send_head_first && got_name && file_exists_p (hstat.local_file))
if (opt.if_modified_since && !send_head_first && got_name && file_exists_p (hstat.local_file, NULL))
{
*dt |= IF_MODIFIED_SINCE;
{
@ -4098,7 +4098,7 @@ http_loop (const struct url *u, struct url *original_url, char **newloc,
}
/* Send preliminary HEAD request if -N is given and we have existing
* destination file or content disposition is enabled. */
else if (opt.content_disposition || file_exists_p (hstat.local_file))
else if (opt.content_disposition || file_exists_p (hstat.local_file, NULL))
send_head_first = true;
}
@ -5103,13 +5103,13 @@ ensure_extension (struct http_stat *hs, const char *ext, int *dt)
strcpy (hs->local_file + local_filename_len, ext);
/* If clobbering is not allowed and the file, as named,
exists, tack on ".NUMBER.html" instead. */
if (!ALLOW_CLOBBER && file_exists_p (hs->local_file))
if (!ALLOW_CLOBBER && file_exists_p (hs->local_file, NULL))
{
int ext_num = 1;
do
sprintf (hs->local_file + local_filename_len,
".%d%s", ext_num++, ext);
while (file_exists_p (hs->local_file));
while (file_exists_p (hs->local_file, NULL));
}
*dt |= ADDED_HTML_EXTENSION;
}

27
src/init.c
View File

@ -566,10 +566,11 @@ wgetrc_env_file_name (void)
char *env = getenv ("WGETRC");
if (env && *env)
{
if (!file_exists_p (env))
file_stats_t flstat;
if (!file_exists_p (env, &flstat))
{
fprintf (stderr, _("%s: WGETRC points to %s, which doesn't exist.\n"),
exec_name, env);
fprintf (stderr, _("%s: WGETRC points to %s, which couldn't be accessed because of error: %s.\n"),
exec_name, env, strerror(flstat.access_err));
exit (WGET_EXIT_GENERIC_ERROR);
}
return xstrdup (env);
@ -597,7 +598,7 @@ wgetrc_user_file_name (void)
if (!file)
return NULL;
if (!file_exists_p (file))
if (!file_exists_p (file, NULL))
{
xfree (file);
return NULL;
@ -630,7 +631,7 @@ wgetrc_file_name (void)
if (home)
{
file = aprintf ("%s/wget.ini", home);
if (!file_exists_p (file))
if (!file_exists_p (file, NULL))
{
xfree (file);
}
@ -658,7 +659,7 @@ static bool setval_internal_tilde (int, const char *, const char *);
there were errors in the file. */
bool
run_wgetrc (const char *file)
run_wgetrc (const char *file, file_stats_t *flstats)
{
FILE *fp;
char *line = NULL;
@ -666,7 +667,7 @@ run_wgetrc (const char *file)
int ln;
int errcnt = 0;
fp = fopen (file, "r");
fp = fopen_stat (file, "r", flstats);
if (!fp)
{
fprintf (stderr, _("%s: Cannot read %s (%s).\n"), exec_name,
@ -722,14 +723,16 @@ void
initialize (void)
{
char *file, *env_sysrc;
file_stats_t flstats;
bool ok = true;
memset(&flstats, 0, sizeof(flstats));
/* Run a non-standard system rc file when the according environment
variable has been set. For internal testing purposes only! */
env_sysrc = getenv ("SYSTEM_WGETRC");
if (env_sysrc && file_exists_p (env_sysrc))
if (env_sysrc && file_exists_p (env_sysrc, &flstats))
{
ok &= run_wgetrc (env_sysrc);
ok &= run_wgetrc (env_sysrc, &flstats);
/* If there are any problems parsing the system wgetrc file, tell
the user and exit */
if (! ok)
@ -743,8 +746,8 @@ or specify a different file using --config.\n"), env_sysrc);
}
/* Otherwise, if SYSTEM_WGETRC is defined, use it. */
#ifdef SYSTEM_WGETRC
else if (file_exists_p (SYSTEM_WGETRC))
ok &= run_wgetrc (SYSTEM_WGETRC);
else if (file_exists_p (SYSTEM_WGETRC, &flstats))
ok &= run_wgetrc (SYSTEM_WGETRC, &flstats);
/* If there are any problems parsing the system wgetrc file, tell
the user and exit */
if (! ok)
@ -771,7 +774,7 @@ or specify a different file using --config.\n"), SYSTEM_WGETRC);
}
else
#endif
ok &= run_wgetrc (file);
ok &= run_wgetrc (file, &flstats);
/* If there were errors processing either `.wgetrc', abort. */
if (!ok)

2
src/init.h
View File

@ -41,6 +41,6 @@ void setoptval (const char *, const char *, const char *);
char *home_dir (void);
void cleanup (void);
void defaults (void);
bool run_wgetrc (const char *file);
bool run_wgetrc (const char *file, file_stats_t *);
#endif /* INIT_H */

10
src/main.c
View File

@ -1381,10 +1381,10 @@ main (int argc, char **argv)
}
else if (strcmp (config_opt->long_name, "config") == 0)
{
bool userrc_ret = true;
userrc_ret &= run_wgetrc (optarg);
file_stats_t flstats;
use_userconfig = true;
if (userrc_ret)
memset(&flstats, 0, sizeof(flstats));
if (file_exists_p(optarg, &flstats) && run_wgetrc (optarg, &flstats))
break;
else
{
@ -1620,7 +1620,7 @@ WARNING: timestamping does nothing in combination with -O. See the manual\n\
for details.\n\n"));
opt.timestamping = false;
}
if (opt.noclobber && file_exists_p(opt.output_document))
if (opt.noclobber && file_exists_p(opt.output_document, NULL))
{
/* Check if output file exists; if it does, exit. */
logprintf (LOG_VERBOSE,
@ -2081,7 +2081,7 @@ only if outputting to a regular file.\n"));
&dt, opt.recursive, iri, true);
}
if (opt.delete_after && filename != NULL && file_exists_p (filename))
if (opt.delete_after && filename != NULL && file_exists_p (filename, NULL))
{
DEBUGP (("Removing file due to --delete-after in main():\n"));
logprintf (LOG_VERBOSE, _("Removing %s.\n"), filename);

8
src/metalink.c
View File

@ -375,6 +375,7 @@ retrieve_from_metalink (const metalink_t* metalink)
metalink_checksum_t **mchksum_ptr, *mchksum;
struct iri *iri;
struct url *url;
file_stats_t flstats;
int url_err;
clean_metalink_string (&mres->url);
@ -490,8 +491,9 @@ retrieve_from_metalink (const metalink_t* metalink)
Bugfix: point output_stream to destname if it exists.
*/
if (!output_stream && file_exists_p (destname))
output_stream = fopen (destname, "ab");
memset(&flstats, 0, sizeof(&flstats));
if (!output_stream && file_exists_p (destname, &flstats))
output_stream = fopen_stat (destname, "ab", &flstats);
}
url_free (url);
iri_free (iri);
@ -901,7 +903,7 @@ gpg_skip_verification:
Note: the file has been downloaded using *_loop. Therefore, it
is not necessary to keep the file for continuated download. */
if (((retr_err != RETROK && !opt.always_rest) || opt.delete_after)
&& destname != NULL && file_exists_p (destname))
&& destname != NULL && file_exists_p (destname, NULL))
{
badhash_or_remove (destname);
}

2
src/retr.c
View File

@ -1141,7 +1141,7 @@ retrieve_from_file (const char *file, bool html, int *count)
if (parsed_url)
url_free (parsed_url);
if (filename && opt.delete_after && file_exists_p (filename))
if (filename && opt.delete_after && file_exists_p (filename, NULL))
{
DEBUGP (("\
Removing file due to --delete-after in retrieve_from_file():\n"));

2
src/url.c
View File

@ -1818,7 +1818,7 @@ url_file_name (const struct url *u, char *replaced_filename)
directory (see `mkalldirs' for explanation). */
if (ALLOW_CLOBBER
&& !(file_exists_p (fname) && !file_non_directory_p (fname)))
&& !(file_exists_p (fname, NULL) && !file_non_directory_p (fname)))
{
unique = fname;
}

162
src/utils.c
View File

@ -45,6 +45,7 @@ as that of the covered work. */
#include <assert.h>
#include <stdarg.h>
#include <locale.h>
#include <errno.h>
#if HAVE_UTIME
# include <sys/types.h>
@ -586,21 +587,43 @@ remove_link (const char *file)
return err;
}
/* Does FILENAME exist? This is quite a lousy implementation, since
it supplies no error codes -- only a yes-or-no answer. Thus it
will return that a file does not exist if, e.g., the directory is
unreadable. I don't mind it too much currently, though. The
proper way should, of course, be to have a third, error state,
other than true/false, but that would introduce uncalled-for
additional complexity to the callers. */
/* Does FILENAME exist? */
bool
file_exists_p (const char *filename)
file_exists_p (const char *filename, file_stats_t *fstats)
{
#ifdef HAVE_ACCESS
return access (filename, F_OK) >= 0;
#else
struct stat buf;
return stat (filename, &buf) >= 0;
#if defined(WINDOWS) || defined(__VMS)
int ret = stat (filename, &buf);
if (ret >= 0)
{
if (fstats != NULL)
fstats->access_err = errno;
}
return ret >= 0;
#else
errno = 0;
if (stat (filename, &buf) == 0 && S_ISREG(buf.st_mode) &&
(((S_IRUSR & buf.st_mode) && (getuid() == buf.st_uid)) ||
((S_IRGRP & buf.st_mode) && group_member(buf.st_gid)) ||
(S_IROTH & buf.st_mode))) {
if (fstats != NULL)
{
fstats->access_err = 0;
fstats->st_ino = buf.st_ino;
fstats->st_dev = buf.st_dev;
}
return true;
}
else
{
if (fstats != NULL)
fstats->access_err = (errno == 0 ? EACCES : errno);
errno = 0;
return false;
}
__builtin_unreachable();
/* NOTREACHED */
#endif
}
@ -668,7 +691,7 @@ unique_name_1 (const char *prefix)
do
number_to_string (template_tail, count++);
while (file_exists_p (template));
while (file_exists_p (template, NULL));
return xstrdup (template);
}
@ -696,7 +719,7 @@ unique_name (const char *file, bool allow_passthrough)
{
/* If the FILE itself doesn't exist, return it without
modification. */
if (!file_exists_p (file))
if (!file_exists_p (file, NULL))
return allow_passthrough ? (char *)file : xstrdup (file);
/* Otherwise, find a numeric suffix that results in unused file name
@ -825,7 +848,7 @@ fopen_excl (const char *fname, int binary)
/* Manually check whether the file exists. This is prone to race
conditions, but systems without O_EXCL haven't deserved
better. */
if (file_exists_p (fname))
if (file_exists_p (fname, NULL))
{
errno = EEXIST;
return NULL;
@ -834,6 +857,113 @@ fopen_excl (const char *fname, int binary)
#endif /* not O_EXCL */
}
/* fopen_stat() assumes that file_exists_p() was called earlier.
file_stats_t passed to this function was returned from file_exists_p()
This is to prevent TOCTTOU race condition.
Details : FIO45-C from https://www.securecoding.cert.org/
Note that for creating a new file, this check is not useful
Input:
fname => Name of file to open
mode => File open mode
fstats => Saved file_stats_t about file that was checked for existence
Returns:
NULL if there was an error
FILE * of opened file stream
*/
FILE *
fopen_stat(const char *fname, const char *mode, file_stats_t *fstats)
{
int fd;
FILE *fp;
struct stat fdstats;
fp = fopen (fname, mode);
if (fp == NULL)
{
logprintf (LOG_NOTQUIET, _("Failed to Fopen file %s\n"), fname);
return NULL;
}
fd = fileno (fp);
if (fd < 0)
{
logprintf (LOG_NOTQUIET, _("Failed to get FD for file %s\n"), fname);
fclose (fp);
return NULL;
}
memset(&fdstats, 0, sizeof(fdstats));
if (fstat (fd, &fdstats) == -1)
{
logprintf (LOG_NOTQUIET, _("Failed to stat file %s, (check permissions)\n"), fname);
fclose (fp);
return NULL;
}
#if !(defined(WINDOWS) || defined(__VMS))
if (fstats != NULL &&
(fdstats.st_dev != fstats->st_dev ||
fdstats.st_ino != fstats->st_ino))
{
/* File changed since file_exists_p() : NOT SAFE */
logprintf (LOG_NOTQUIET, _("File %s changed since the last check. Security check failed."), fname);
fclose (fp);
return NULL;
}
#endif
return fp;
}
/* open_stat assumes that file_exists_p() was called earlier to save file_stats
file_stats_t passed to this function was returned from file_exists_p()
This is to prevent TOCTTOU race condition.
Details : FIO45-C from https://www.securecoding.cert.org/
Note that for creating a new file, this check is not useful
Input:
fname => Name of file to open
flags => File open flags
mode => File open mode
fstats => Saved file_stats_t about file that was checked for existence
Returns:
-1 if there was an error
file descriptor of opened file stream
*/
int
open_stat(const char *fname, int flags, mode_t mode, file_stats_t *fstats)
{
int fd;
struct stat fdstats;
fd = open (fname, flags, mode);
if (fd < 0)
{
logprintf (LOG_NOTQUIET, _("Failed to open file %s, reason :%s\n"), fname, strerror(errno));
return -1;
}
memset(&fdstats, 0, sizeof(fdstats));
if (fstat (fd, &fdstats) == -1)
{
logprintf (LOG_NOTQUIET, _("Failed to stat file %s, error: %s\n"), fname, strerror(errno));
return -1;
}
#if !(defined(WINDOWS) || defined(__VMS))
if (fstats != NULL &&
(fdstats.st_dev != fstats->st_dev ||
fdstats.st_ino != fstats->st_ino))
{
/* File changed since file_exists_p() : NOT SAFE */
logprintf (LOG_NOTQUIET, _("Trying to open file %s but it changed since last check. Security check failed."), fname);
close (fd);
return -1;
}
#endif
return fd;
}
/* Create DIRECTORY. If some of the pathname components of DIRECTORY
are missing, create them first. In case any mkdir() call fails,
return its error status. Returns 0 on successful completion.
@ -862,7 +992,7 @@ make_directory (const char *directory)
/* Check whether the directory already exists. Allow creation of
of intermediate directories to fail, as the initial path components
are not necessarily directories! */
if (!file_exists_p (dir))
if (!file_exists_p (dir, NULL))
ret = mkdir (dir, 0777);
else
ret = 0;

10
src/utils.h
View File

@ -78,15 +78,23 @@ void fork_to_background (void);
char *aprintf (const char *, ...) GCC_FORMAT_ATTR (1, 2);
char *concat_strings (const char *, ...);
typedef struct file_stat_s {
int access_err; /* Error in accecssing file : Not present vs permission */
ino_t st_ino; /* st_ino from stats() on the file before open() */
dev_t st_dev; /* st_dev from stats() on the file before open() */
} file_stats_t;
void touch (const char *, time_t);
int remove_link (const char *);
bool file_exists_p (const char *);
bool file_exists_p (const char *, file_stats_t *);
bool file_non_directory_p (const char *);
wgint file_size (const char *);
int make_directory (const char *);
char *unique_name (const char *, bool);
FILE *unique_create (const char *, bool, char **);
FILE *fopen_excl (const char *, int);
FILE *fopen_stat (const char *, const char *, file_stats_t *);
int open_stat (const char *, int, mode_t, file_stats_t *);
char *file_merge (const char *, const char *);
int fnmatch_nocase (const char *, const char *, int);