Re: [PATCH v7 2/5] setup: sanity check file size in read_gitfile_gently

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 14, 2015, 03:42 UTC
Message-ID: <CAPig+cTQKxD9WRV3spVce=SfeECo4XR_ujZ1sGecuhTL-3DeRw@mail.gmail.com>
In-Reply-To: <1433874279-30720-3-git-send-email-erik.elfstrom@gmail.com>

On Tue, Jun 9, 2015 at 2:24 PM, Erik Elfström <erik.elfstrom@gmail.com> wrote:

Show 18 quoted lines

> read_gitfile_gently will allocate a buffer to fit the entire file that
> should be read. Add a sanity check of the file size before opening to
> avoid allocating a potentially huge amount of memory if we come across
> a large file that someone happened to name ".git". The limit is set to
> a sufficiently unreasonable size that should never be exceeded by a
> genuine .git file.
>
> Signed-off-by: Erik Elfström <erik.elfstrom@gmail.com>
> ---
> diff --git a/setup.c b/setup.c
> index 4748b63..e76955e 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -414,6 +414,7 @@ static void update_linked_gitdir(const char *gitfile, const char *gitdir)
>   */
>  const char *read_gitfile_gently(const char *path, int *return_error_code)
>  {
> +       static const int one_MB = 1 << 20;

This variable name doesn't convey much about its purpose, and introduces a bit of maintenance burden if the limit is some day changed. Perhaps "sane_size_limit" or something even more descriptive (and/or terse) would be better.

Show 25 quoted lines

>         int error_code = 0;
>         char *buf = NULL;
>         char *dir = NULL;
> @@ -430,6 +431,10 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
>                 error_code = READ_GITFILE_ERR_NOT_A_FILE;
>                 goto cleanup_return;
>         }
> +       if (st.st_size > one_MB) {
> +               error_code = READ_GITFILE_ERR_TOO_LARGE;
> +               goto cleanup_return;
> +       }
>         fd = open(path, O_RDONLY);
>         if (fd < 0) {
>                 error_code = READ_GITFILE_ERR_OPEN_FAILED;
> @@ -489,6 +494,8 @@ cleanup_return:
>                         return NULL;
>                 case READ_GITFILE_ERR_OPEN_FAILED:
>                         die_errno("Error opening '%s'", path);
> +               case READ_GITFILE_ERR_TOO_LARGE:
> +                       die("Too large to be a .git file: '%s'", path);
>                 case READ_GITFILE_ERR_READ_FAILED:
>                         die("Error reading %s", path);
>                 case READ_GITFILE_ERR_INVALID_FORMAT:
> --
> 2.4.3.373.gc496bfb