Rare Windows crash due to potentially dangling `data->state.mimepost` pointer #12608

tferguson7337 · 2023-12-29T15:38:44Z

I did this

First time tracing through curl code and I'm unfamiliar with curl's workflow, so apologies in advanced if my analysis here is lacking or inaccurate.

From what I can tell, a somewhat recent change was made via 74b87a8, which added two new formp and mimepost pointers to UrlState. There appears to be a case where the mimepost pointer can be set to formp, but left dangling after formp is freed and cleared.

Crashing stack (bottom of stack w/ non-curl calls omitted for privacy):

 # Child-SP          RetAddr               Call Site
00 000000bf`012fe038 00007ff9`5f27235f     ntdll!LdrpICallHandler+0xf
01 000000bf`012fe040 00007ff9`5f2214a4     ntdll!RtlpExecuteHandlerForException+0xf
02 000000bf`012fe070 00007ff9`5f270e8e     ntdll!RtlDispatchException+0x244
03 000000bf`012fe780 00007ff9`5f25c67e     ntdll!KiUserExceptionDispatch+0x2e
04 000000bf`012fee98 00007ff6`bceb0696     ntdll!LdrpDispatchUserCallTarget+0xe
05 000000bf`012feea0 00007ff6`bceaf659     <my_exe>!mime_part_rewind+0x56 [...\curl\lib\mime.c @ 1078] 
06 000000bf`012feed0 00007ff6`bcea9984     <my_exe>!Curl_mime_rewind+0x9 [...\curl\lib\mime.c @ 1611] 
07 000000bf`012fef00 00007ff6`bcea7a39     <my_exe>!readrewind+0x1f4 [...\curl\lib\multi.c @ 1797] 
08 000000bf`012fef30 00007ff6`bcea6693     <my_exe>!multi_runsingle+0x5d9 [...\curl\lib\multi.c @ 2116] 
09 000000bf`012ff040 00007ff6`bce9ec07     <my_exe>!curl_multi_perform+0xa3 [...\curl\lib\multi.c @ 2741] 
0a (Inline Function) --------`--------     <my_exe>!easy_transfer+0x37 [...\curl\lib\easy.c @ 679] 
0b (Inline Function) --------`--------     <my_exe>!easy_perform+0x12d [...\curl\lib\easy.c @ 769] 
0c 000000bf`012ff0c0 00007ff6`bc9935aa     <my_exe>!curl_easy_perform+0x137 [...\curl\lib\easy.c @ 788] 
...

data->state content, showing null formp and (dangling?) mimepost at time of crash:

0:067> ?? &data->state
struct UrlState * 0x00000285`9371ba88
...
   +0x5b8 mimepost         : 0x00000285`95e772a0 curl_mimepart
   +0x5c0 formp            : (null) 
...

data->state.mimepost content at time of crash, which appears to be in the midst of getting modified by something else at this point - content is incoherent:

0:067> dt -r2 curl_mimepart 0x00000285`95e772a0
<my_exe>!curl_mimepart
   +0x000 parent           : 0x00000000`00010000 curl_mime
      +0x000 parent           : ???? 
      +0x008 firstpart        : ???? 
      +0x010 lastpart         : ???? 
      +0x018 boundary         : [47]  "--- memory read error at address 0x00000000`00010018 ---"
      +0x048 state            : mime_state
         +0x000 state            : ??
         +0x008 ptr              : ???? 
         +0x010 offset           : ??
   +0x008 nextpart         : 0x00000000`00000004 curl_mimepart
      +0x000 parent           : ???? 
      +0x008 nextpart         : ???? 
      +0x010 kind             : ??
      +0x014 flags            : ??
      +0x018 data             : ???? 
      +0x020 readfunc         : ???? 
      +0x028 seekfunc         : ???? 
      +0x030 freefunc         : ???? 
      +0x038 arg              : ???? 
      +0x040 fp               : ???? 
      +0x048 curlheaders      : ???? 
      +0x050 userheaders      : ???? 
      +0x058 mimetype         : ???? 
      +0x060 filename         : ???? 
      +0x068 name             : ???? 
      +0x070 datasize         : ??
      +0x078 state            : mime_state
         +0x000 state            : ??
         +0x008 ptr              : ???? 
         +0x010 offset           : ??
      +0x090 encoder          : ???? 
      +0x098 encstate         : mime_encoder_state
         +0x000 pos              : ??
         +0x008 bufbeg           : ??
         +0x010 bufend           : ??
         +0x018 buf              : [256]  "--- memory read error at address 0x00000000`000000b4 ---"
      +0x1b0 lastreadstatus   : ??
   +0x010 kind             : 0 ( MIMEKIND_NONE )
   +0x014 flags            : 0
   +0x018 data             : (null) 
   +0x020 readfunc         : (null) 
   +0x028 seekfunc         : (null) 
   +0x030 freefunc         : 0x00000285`95e77328     void  +28595e77328
   +0x038 arg              : 0x00000285`92163e50 Void
   +0x040 fp               : 0x00000007`00000003 _iobuf
      +0x000 _Placeholder     : ???? 
   +0x048 curlheaders      : 0x00000000`00020002 curl_slist
      +0x000 data             : ???? 
      +0x008 next             : ???? 
   +0x050 userheaders      : (null) 
   +0x058 mimetype         : 0x00000285`95e77318  "???"
   +0x060 filename         : 0x00000285`921789c6  "???"
   +0x068 name             : 0x0000000e`0000000e  "--- memory read error at address 0x0000000e`0000000e ---"
   +0x070 datasize         : 0n8589934623
   +0x078 state            : mime_state
      +0x000 state            : 3 ( MIMESTATE_EOH )
      +0x008 ptr              : 0x00000000`0000000e Void
      +0x010 offset           : 0n256
   +0x090 encoder          : (null) 
   +0x098 encstate         : mime_encoder_state
      +0x000 pos              : 0
      +0x008 bufbeg           : 0
      +0x010 bufend           : 0x00000285`91a56d00
      +0x018 buf              : [256]  ""
   +0x1b0 lastreadstatus   : 1

It's hard to 100% confirm from the crash dump, but I believe the following occurred:

In the middle of attempting a previous HTTP-POST, we appear to hit a network issue (there's hints in the dump of us getting back CURLE_SSL_CONNECT_ERROR in a previous operation using this easy curl handle).
I believe we made it far enough for formp to get allocated in Curl_http_body, and for mimepost to be assigned to:

  case HTTPREQ_POST_FORM:
    /* Convert the form structure into a mime structure, then keep
       the conversion */
    if(!data->state.formp) {
      data->state.formp = calloc(sizeof(curl_mimepart), 1);
      if(!data->state.formp)
        return CURLE_OUT_OF_MEMORY;
      Curl_mime_cleanpart(data->state.formp);
      result = Curl_getformdata(data, data->state.formp, data->set.httppost,
                                data->state.fread_func);
      if(result)
        return result;
      data->state.mimepost = data->state.formp;

But hit network issues later on when doing the request that resulted in data->state.rewindbeforesend to be set to TRUE, potentially in Curl_retry_request:

    if((conn->handler->protocol&PROTO_FAMILY_HTTP) &&
       data->req.writebytecount) {
      data->state.rewindbeforesend = TRUE;
      infof(data, "state.rewindbeforesend = TRUE");
    }

We later reuse the same easy curl handle to attempt a final "goodbye" POST.

This again uses CURLOPT_HTTPPOST, which frees and clears data->state.formp (note that data->state.mimepost is not cleared here):

  case CURLOPT_HTTPPOST:
    /*
     * Set to make us do HTTP POST. Legacy API-style.
     */
    data->set.httppost = va_arg(param, struct curl_httppost *);
    data->set.method = HTTPREQ_POST_FORM;
    data->set.opt_no_body = FALSE; /* this is implied */
    Curl_mime_cleanpart(data->state.formp);
    Curl_safefree(data->state.formp);
    break;

During MSTATE_PROTOCONNECT, the data->state.rewindbeforesend flag check passes, so rewind is attempted. It's not clear to me if this flag being set is carry over from the previous failed POST attempt or was set during a previous MSTATE (e.g., MSTATE_CONNECT):

    case MSTATE_PROTOCONNECT:
      if(data->state.rewindbeforesend)
        result = readrewind(data);  <-- We attempt readrewind call here.

readrewind is initially primed to use &data->set.mimepost, but has a conditional check that gets met to use data->state.mimepost instead:

static CURLcode readrewind(struct Curl_easy *data)
{
  curl_mimepart *mimepart = &data->set.mimepost;
  ...
    if(data->conn->handler->protocol & PROTO_FAMILY_HTTP) {
    if(data->state.mimepost)
      mimepart = data->state.mimepost; // <--- use UrlState mimepost now
  }
  ...
    if(data->set.postfields ||
     (data->state.httpreq == HTTPREQ_GET) ||
     (data->state.httpreq == HTTPREQ_HEAD))
    ; /* no need to rewind */
  else if(data->state.httpreq == HTTPREQ_POST_MIME ||
          data->state.httpreq == HTTPREQ_POST_FORM) {
    CURLcode result = Curl_mime_rewind(mimepart); <-- Crash in here
    if(result) {
      failf(data, "Cannot rewind mime/post data");
      return result;
    }
  }

mime_part_rewind uses the (presumably dangling) pointer, which at this point is being reused by something else, the various checks (including for non-null seekfunc) happen to pass as the memory the dangling pointer references is being modified:

static int mime_part_rewind(curl_mimepart *part)
{
  int res = CURL_SEEKFUNC_OK;
  enum mimestate targetstate = MIMESTATE_BEGIN;

  if(part->flags & MIME_BODY_ONLY)
    targetstate = MIMESTATE_BODY;
  cleanup_encoder_state(&part->encstate);
  if(part->state.state > targetstate) {
    res = CURL_SEEKFUNC_CANTSEEK;
    if(part->seekfunc) {
      res = part->seekfunc(part->arg, (curl_off_t) 0, SEEK_SET);  <--- crash here when attempting to call part->seekfunc
...
}

I expected the following

No crash.

Perhaps data->state.mimepost should be cleared when handling CURLOPT_HTTPPOST where data->state.formp is freed/cleared, like so?

  case CURLOPT_HTTPPOST:
    /*
     * Set to make us do HTTP POST. Legacy API-style.
     */
    data->set.httppost = va_arg(param, struct curl_httppost *);
    data->set.method = HTTPREQ_POST_FORM;
    data->set.opt_no_body = FALSE; /* this is implied */
    if (data->state.mimepart == data->state.formp) // ?? Is check needed or always clear ??
    {
          data->state.mimepart = NULL;
    }
    Curl_mime_cleanpart(data->state.formp);
    Curl_safefree(data->state.formp);
    break;

curl/libcurl version

curl 8.4.0

operating system

Windows 10 x64, 21H2

The text was updated successfully, but these errors were encountered:

bagder · 2023-12-29T23:11:07Z

I don't believe the check is necessary and it can be cleared unconditionally. Does this fix the problem for you?

diff --git a/lib/setopt.c b/lib/setopt.c
index 460cb32e7..e13432334 100644
--- a/lib/setopt.c
+++ b/lib/setopt.c
@@ -679,10 +679,11 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param)
     data->set.httppost = va_arg(param, struct curl_httppost *);
     data->set.method = HTTPREQ_POST_FORM;
     data->set.opt_no_body = FALSE; /* this is implied */
     Curl_mime_cleanpart(data->state.formp);
     Curl_safefree(data->state.formp);
+    data->state.mimepost = NULL;
     break;
 #endif
 
 #if !defined(CURL_DISABLE_AWS)
   case CURLOPT_AWS_SIGV4:
@@ -986,10 +987,11 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param)
       data->set.method = HTTPREQ_POST_MIME;
       data->set.opt_no_body = FALSE; /* this is implied */
 #ifndef CURL_DISABLE_FORM_API
       Curl_mime_cleanpart(data->state.formp);
       Curl_safefree(data->state.formp);
+      data->state.mimepost = NULL;
 #endif
     }
     break;
 
   case CURLOPT_MIME_OPTIONS:

tferguson7337 · 2024-01-02T13:14:31Z

Based on my read of the code I think it should, but I can't really confirm directly. We've only seen this crash once, in one of our (many) nightly automation runs, which haven't had any issues like this since we first updated to curl 8.4.0 back in October.

A precaution to avoid a possible dangling pointer left behind. Reported-by: Thomas Ferguson Fixes #12608

bagder · 2024-01-02T15:39:35Z

Ok, then let's hope this fixes it and move forward with this take. I made #12621.

tferguson7337 · 2024-01-02T16:01:24Z

Thanks @bagder, 🤞

bagder added the crash label Dec 29, 2023

bagder added a commit that referenced this issue Jan 2, 2024

setopt: clear mimepost when formp is freed

9a9ace9

A precaution to avoid a possible dangling pointer left behind. Reported-by: Thomas Ferguson Fixes #12608

bagder mentioned this issue Jan 2, 2024

setopt: clear mimepost when formp is freed #12621

Closed

bagder closed this as completed in eeda18b Jan 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rare Windows crash due to potentially dangling `data->state.mimepost` pointer #12608

Rare Windows crash due to potentially dangling `data->state.mimepost` pointer #12608

tferguson7337 commented Dec 29, 2023

bagder commented Dec 29, 2023

tferguson7337 commented Jan 2, 2024

bagder commented Jan 2, 2024

tferguson7337 commented Jan 2, 2024

Rare Windows crash due to potentially dangling data->state.mimepost pointer #12608

Rare Windows crash due to potentially dangling data->state.mimepost pointer #12608

Comments

tferguson7337 commented Dec 29, 2023

I did this

I expected the following

curl/libcurl version

operating system

bagder commented Dec 29, 2023

tferguson7337 commented Jan 2, 2024

bagder commented Jan 2, 2024

tferguson7337 commented Jan 2, 2024

Rare Windows crash due to potentially dangling `data->state.mimepost` pointer #12608

Rare Windows crash due to potentially dangling `data->state.mimepost` pointer #12608