bug-wget
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-wget] Fwd: PATCH: bugs 20369 and 20389


From: Vijo Cherian
Subject: Re: [Bug-wget] Fwd: PATCH: bugs 20369 and 20389
Date: Sun, 5 Mar 2017 21:30:12 -0800

Thank you Darshit. Please find the patch attached to this email.
This should address all your concerns about the patch.

Best,
Vijo.

On Sat, Mar 4, 2017 at 9:26 PM, Darshit Shah <address@hidden> wrote:

> Hi Vijo,
>
> Could you please send the patches as an attachment generated by `git
> format-patch`? Gmail more often than not tends to break line endings making
> it almost impossible to test the patches.
>
> I will currently let Eli look into the Windows and VMS compatibility for
> safe_{f,}open() methods. However, I have two minor nitpicks about this
> patch:
>
> 1. The log messages seem more in line with DEBUG than NOTQUIET. Any
>  specific reasons why you chose this log level?
> 2. Why make the change to only one test case? Please remove the --debug
>  switch you added to it. Would it be possible to write a testcase that
>  actually shows this TOCTOU attack in practice? I guess not, but if   you
> have any ideas, do let us know.
>
>
> * Vijo Cherian <address@hidden> [170304 19:51]:
>
> Thanks again Eli. Here is my revised patch, that addresses all except one
>> of your concerns.
>> One concern I didn't address is that of making file_exists_p() call inside
>> fopen_stat().
>> That would require a lot more of code surgery than I should as I get
>> familiar with wget code.
>> But I can take on a cleanup task at a later time.
>>
>> Inlined patch this time.
>>
>> Best,
>> Vijo.
>>
>>
>>
>> diff --git a/src/ftp.c b/src/ftp.c
>> index 2f2866c..a790c39 100644
>> --- a/src/ftp.c
>> +++ b/src/ftp.c
>> @@ -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);
>>             }
>> diff --git a/src/hsts.c b/src/hsts.c
>> index 91cc527..67bdde6 100644
>> --- a/src/hsts.c
>> +++ b/src/hsts.c
>> @@ -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))
>>             {
>> diff --git a/src/http.c b/src/http.c
>> index d2c5c77..f42963f 100644
>> --- a/src/http.c
>> +++ b/src/http.c
>> @@ -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;
>>     }
>> diff --git a/src/init.c b/src/init.c
>> index e6aa673..63c89b5 100644
>> --- a/src/init.c
>> +++ b/src/init.c
>> @@ -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)
>> diff --git a/src/init.h b/src/init.h
>> index 1969cc7..a57ca78 100644
>> --- a/src/init.h
>> +++ b/src/init.h
>> @@ -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 */
>> diff --git a/src/main.c b/src/main.c
>> index 311a91f..a1d5714 100644
>> --- a/src/main.c
>> +++ b/src/main.c
>> @@ -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);
>> diff --git a/src/metalink.c b/src/metalink.c
>> index 904f9f3..6bee3c9 100644
>> --- a/src/metalink.c
>> +++ b/src/metalink.c
>> @@ -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);
>>         }
>> diff --git a/src/retr.c b/src/retr.c
>> index 5ba744f..0cf438e 100644
>> --- a/src/retr.c
>> +++ b/src/retr.c
>> @@ -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"));
>> diff --git a/src/url.c b/src/url.c
>> index 8f8ff0b..01098b7 100644
>> --- a/src/url.c
>> +++ b/src/url.c
>> @@ -1807,7 +1807,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;
>>     }
>> diff --git a/src/utils.c b/src/utils.c
>> index db89ae1..3acc78d 100644
>> --- a/src/utils.c
>> +++ b/src/utils.c
>> @@ -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;
>> diff --git a/src/utils.h b/src/utils.h
>> index aaac730..8acb258 100644
>> --- a/src/utils.h
>> +++ b/src/utils.h
>> @@ -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);
>>
>>
>> On Sat, Mar 4, 2017 at 7:30 AM, Eli Zaretskii <address@hidden> wrote:
>>
>> > From: Vijo Cherian <address@hidden>
>>> > Date: Fri, 3 Mar 2017 11:33:05 -0800
>>> > Cc: address@hidden
>>> >
>>> > bool
>>> > file_exists_p (const char *filename, file_stats_t *fstats)
>>> > {
>>> >   struct stat buf;
>>> >
>>> > #if defined(WINDOWS) || defined(__VMS)
>>> >     return stat (filename, &buf) >= 0;
>>> > #else
>>>
>>> This leaves fstats untouched on Windows.  At least access_err should
>>> be set, I think.
>>>
>>>
> --
> Thanking You,
> Darshit Shah
> PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6
>

Attachment: 0001-BUGFIX.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]