FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 08-17-2011, 12:37 PM
Dave Reisner
 
Default Avoid stat() on NULL path in curl_download_internal()

On Wed, Aug 17, 2011 at 10:15:16AM +0200, Lukas Fleischer wrote:
> stat()'s behaviour is undefined if the first argument is NULL and might
> be prone to segfault. Add an additional check to skip the stat()
> invocation if no destfile is used.
>
> Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de>
> ---
> lib/libalpm/dload.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 5a63e48..731d807 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -260,7 +260,7 @@ static int curl_download_internal(struct dload_payload *payload,
> curl_easy_setopt(handle->curl, CURLOPT_USERAGENT, useragent);
> }
>
> - if(!payload->allow_resume && !payload->force && stat(destfile, &st) == 0) {
> + if(!payload->allow_resume && !payload->force && destfile && stat(destfile, &st) == 0) {
> /* start from scratch, but only download if our local is out of date. */
> curl_easy_setopt(handle->curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_IFMODSINCE);
> curl_easy_setopt(handle->curl, CURLOPT_TIMEVALUE, (long)st.st_mtime);
> --
> 1.7.6
>
>

We already check for destfile being NULL earlier, when we try to create it, line 210ish.

d
 
Old 08-17-2011, 01:05 PM
Lukas Fleischer
 
Default Avoid stat() on NULL path in curl_download_internal()

On Wed, Aug 17, 2011 at 08:37:23AM -0400, Dave Reisner wrote:
> On Wed, Aug 17, 2011 at 10:15:16AM +0200, Lukas Fleischer wrote:
> > stat()'s behaviour is undefined if the first argument is NULL and might
> > be prone to segfault. Add an additional check to skip the stat()
> > invocation if no destfile is used.
> >
> > Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de>
> > ---
> > lib/libalpm/dload.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> > index 5a63e48..731d807 100644
> > --- a/lib/libalpm/dload.c
> > +++ b/lib/libalpm/dload.c
> > @@ -260,7 +260,7 @@ static int curl_download_internal(struct dload_payload *payload,
> > curl_easy_setopt(handle->curl, CURLOPT_USERAGENT, useragent);
> > }
> >
> > - if(!payload->allow_resume && !payload->force && stat(destfile, &st) == 0) {
> > + if(!payload->allow_resume && !payload->force && destfile && stat(destfile, &st) == 0) {
> > /* start from scratch, but only download if our local is out of date. */
> > curl_easy_setopt(handle->curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_IFMODSINCE);
> > curl_easy_setopt(handle->curl, CURLOPT_TIMEVALUE, (long)st.st_mtime);
> > --
> > 1.7.6
> >
> >
>
> We already check for destfile being NULL earlier, when we try to create it, line 210ish.

Yeah, we do not check that in the else branch though.
 
Old 08-17-2011, 01:24 PM
Dave Reisner
 
Default Avoid stat() on NULL path in curl_download_internal()

On Wed, Aug 17, 2011 at 03:05:55PM +0200, Lukas Fleischer wrote:
> On Wed, Aug 17, 2011 at 08:37:23AM -0400, Dave Reisner wrote:
> > On Wed, Aug 17, 2011 at 10:15:16AM +0200, Lukas Fleischer wrote:
> > > stat()'s behaviour is undefined if the first argument is NULL and might
> > > be prone to segfault. Add an additional check to skip the stat()
> > > invocation if no destfile is used.
> > >
> > > Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de>
> > > ---
> > > lib/libalpm/dload.c | 2 +-
> > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> > > index 5a63e48..731d807 100644
> > > --- a/lib/libalpm/dload.c
> > > +++ b/lib/libalpm/dload.c
> > > @@ -260,7 +260,7 @@ static int curl_download_internal(struct dload_payload *payload,
> > > curl_easy_setopt(handle->curl, CURLOPT_USERAGENT, useragent);
> > > }
> > >
> > > - if(!payload->allow_resume && !payload->force && stat(destfile, &st) == 0) {
> > > + if(!payload->allow_resume && !payload->force && destfile && stat(destfile, &st) == 0) {
> > > /* start from scratch, but only download if our local is out of date. */
> > > curl_easy_setopt(handle->curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_IFMODSINCE);
> > > curl_easy_setopt(handle->curl, CURLOPT_TIMEVALUE, (long)st.st_mtime);
> > > --
> > > 1.7.6
> > >
> > >
> >
> > We already check for destfile being NULL earlier, when we try to create it, line 210ish.
>
> Yeah, we do not check that in the else branch though.
>

And this is what I get for looking at this on the wrong branch. Yeah,
this makes sense.
 

Thread Tools




All times are GMT. The time now is 09:48 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org