Re: [Qemu-devel] [PATCH] curl: refuse to open URL from HTTP server without range support

[Fam Zheng <famz@redhat.com>]

On Tue, 06/11 09:40, Stefan Hajnoczi wrote:
> On Tue, Jun 11, 2013 at 11:15:15AM +0800, Fam Zheng wrote:
> > On Mon, Jun 10, 2013 at 5:21 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > > On Sun, Jun 09, 2013 at 10:34:54AM +0800, Fam Zheng wrote:
> > >> @@ -110,14 +111,14 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
> > >>      return 0;
> > >>  }
> > >>
> > >> -static size_t curl_size_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
> > >> +static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
> > >>  {
> > >> -    CURLState *s = ((CURLState*)opaque);
> > >> +    BDRVCURLState *s = opaque;
> > >>      size_t realsize = size * nmemb;
> > >> -    size_t fsize;
> > >> +    const char *accept_line = "Accept-Ranges: bytes";
> > >>
> > >> -    if(sscanf(ptr, "Content-Length: %zd", &fsize) == 1) {
> > >> -        s->s->len = fsize;
> > >> +    if (strncmp((char *)ptr, accept_line, strlen(accept_line)) == 0) {
> > >> +        s->accept_range = true;
> > >>      }
> > >
> > > This still assumes ptr is NUL-terminated.  You need to pass size * nmemb
> > > instead of strlen(accept_line).
> > >
> > OK, the case is so corner, only when :
> > - realsize < strlen(accept_line) and
> > - ptr is the first part of  accept_line, without NUL-termination
> > strncpm will possibly access no more than (strlen(accept_line) -
> > realsize) bytes after ptr buffer.
> > 
> > I'll need to check if realsize >= strlen(accept_line), not passing realsize.
> 
> You can just pass size * nmemb because strncmp() does check for NUL in
> both strings.  Therefore strlen(accept_line) is not needed - you know
> accept_line is NUL-terminated.
> 

No, e.g. size * nmemb is 5, and *ptr is "Conte", passing size * nmemb to
strncmp gets zero. We need to:
    * Ensure size * nmemb is no less than needed
    * Only compare needed, not whole (first strlen(accept_line) bytes).

-- 
Fam