Bugs item #2958474, was opened at 2010-02-25 03:30
Message generated for change (Comment added) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2958474&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: crash
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Pat Ray (patray)
Assigned to: Daniel Stenberg (bagder)
Summary: trailer off-by-one problem in chunk parser

Initial Comment:
I'm using libcurl 7.19.7 and 7.20.0, the latter compiled from sources. I'm seeing the crash on Win32 (XP, Server 2008) in both version of libcurl.

I'm getting a crash in curl_easy_destroy on a curl handle that was used to read a chunked transfer in HTTP. The problem turned out to be a fencepost error in http_chunks.c. Here's the relevant section of code in 7.20.0:

    case CHUNK_TRAILER:
      /* conn->trailer is assumed to be freed in url.c on a
         connection basis */
      if(conn->trlPos >= conn->trlMax) {
        char *ptr;
        if(conn->trlMax) {
          conn->trlMax *= 2;
          ptr = realloc(conn->trailer,conn->trlMax);
        }
        else {
          conn->trlMax=128;
          ptr = malloc(conn->trlMax);
        }
        if(!ptr)
          return CHUNKE_OUT_OF_MEMORY;
        conn->trailer = ptr;
      }
      conn->trailer[conn->trlPos++]=*datap;

      if(*datap == 0x0d)
        ch->state = CHUNK_TRAILER_CR;
      else {
        datap++;
        length--;
      }
      break;

    case CHUNK_TRAILER_CR:
      if(*datap == 0x0d) {
        ch->state = CHUNK_TRAILER_POSTCR;
        datap++;
        length--;
      }
      else
        return CHUNKE_BAD_CHUNK;
      break;

    case CHUNK_TRAILER_POSTCR:
      if(*datap == 0x0a) {
        conn->trailer[conn->trlPos++]=0x0a;
        conn->trailer[conn->trlPos]=0;
        if(conn->trlPos==2) {
          ch->state = CHUNK_STOP;
          length--;

          /*
           * Note that this case skips over the final STOP states since we've
           * already read the final CRLF and need to return
           */

          ch->dataleft = length;

Note that if the length of the trailer is 127 bytes, we'll write one off the end of the ptr returned in the malloc statement since the CHUNK_TRAILER_POSTCR writes two bytes into that buffer even when trlPos is 127.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2010-02-26 23:55

Message:
Thanks for the report, this problem is now fixed in CVS!

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2958474&group_id=976
Received on 2010-02-26