On Sun, Oct 20, 2013 at 07:47:06AM +0200, Torsten Bögershausen wrote:
> (may be s/path is too big/path is too long/ ?)
>
> On 19.10.13 12:52, Antoine Pelisse wrote:
> > Currently, most buffers created with PATH_MAX length, are not checked
> > when being written, and can overflow if PATH_MAX is not big enough to
> > hold the path.
> >
> > Fix that by using strlcpy() where strcpy() was used, and also run some
> > extra checks when copy is done with memcpy().
> >
> > Reported-by: Wataru Noguchi <wnoguchi.0...@gmail.com>
> > Signed-off-by: Antoine Pelisse <apeli...@gmail.com>
> > ---
> > diff --git a/abspath.c b/abspath.c
> > index 64adbe2..0e60ba4 100644
> > --- a/abspath.c
> > +++ b/abspath.c
> > @@ -216,11 +216,15 @@ const char *absolute_path(const char *path)
> > const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
> > {
> > static char path[PATH_MAX];
Why do you need static there?
> > +
> > + if (pfx_len > PATH_MAX)
> I think this should be
> if (pfx_len > PATH_MAX-1) /* Keep 1 char for '\0'
> > + die("Too long prefix path: %s", pfx);
> > +
> > #ifndef GIT_WINDOWS_NATIVE
> > if (!pfx_len || is_absolute_path(arg))
> > return arg;
> > memcpy(path, pfx, pfx_len);
> > - strcpy(path + pfx_len, arg);
> > + strlcpy(path + pfx_len, arg, PATH_MAX - pfx_len);
>
> I'm not sure how to handle overlong path in general, there are several ways:
> a) Silently overwrite memory (with help of memcpy() and/or strcpy()
> b) Silently shorten the path using strlcpy() instead of strcpy()
> c) Avoid the overwriting and call die().
> d) Prepare a longer buffer using xmalloc()
>
There is also
e) modify allocation to place write protected page after buffer end.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
