Unverified Commit 7f1140c8 authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

multi_cleanup: call DONE on handles that never got that 

... fixes a memory leak with at least IMAP when remove_handle is never
called and the transfer is abruptly just abandoned early.

Test 1552 added to verify

Detected by OSS-fuzz
Assisted-by: Max Dymond
Closes #1954
parent 454dae00

lib/multi.c

+21 −18
Original line number Diff line number Diff line
@@ -2224,6 +2224,27 @@ CURLMcode curl_multi_cleanup(struct Curl_multi *multi) 


    multi->type = 0; /* not good anymore */



    /* Firsrt remove all remaining easy handles */

    data = multi->easyp;

    while(data) {

      nextdata = data->next;

      if(!data->state.done && data->easy_conn)

        /* if DONE was never called for this handle */

        (void)multi_done(&data->easy_conn, CURLE_OK, TRUE);

      if(data->dns.hostcachetype == HCACHE_MULTI) {

        /* clear out the usage of the shared DNS cache */

        Curl_hostcache_clean(data, data->dns.hostcache);

        data->dns.hostcache = NULL;

        data->dns.hostcachetype = HCACHE_NONE;

      }



      /* Clear the pointer to the connection cache */

      data->state.conn_cache = NULL;

      data->multi = NULL; /* clear the association */



      data = nextdata;

    }



    /* Close all the connections in the connection cache */

    close_all_connections(multi);
@@ -2243,24 +2264,6 @@ CURLMcode curl_multi_cleanup(struct Curl_multi *multi) 
    Curl_llist_destroy(&multi->msglist, NULL);

    Curl_llist_destroy(&multi->pending, NULL);



    /* remove all easy handles */

    data = multi->easyp;

    while(data) {

      nextdata = data->next;

      if(data->dns.hostcachetype == HCACHE_MULTI) {

        /* clear out the usage of the shared DNS cache */

        Curl_hostcache_clean(data, data->dns.hostcache);

        data->dns.hostcache = NULL;

        data->dns.hostcachetype = HCACHE_NONE;

      }



      /* Clear the pointer to the connection cache */

      data->state.conn_cache = NULL;

      data->multi = NULL; /* clear the association */



      data = nextdata;

    }



    Curl_hash_destroy(&multi->hostcache);



    /* Free the blacklists by setting them to NULL */

tests/data/Makefile.inc

+1 −1
Original line number Diff line number Diff line
@@ -169,7 +169,7 @@ test1520 test1521 \ 
test1525 test1526 test1527 test1528 test1529 test1530 test1531 test1532 \

test1533 test1534 test1535 test1536 test1537 test1538 \

test1540 \

test1550 test1551 \

test1550 test1551 test1552 \

test1600 test1601 test1602 test1603 test1604 test1605 test1606 \

\

test1700 test1701 test1702 \

tests/data/test1552

0 → 100644
+52 −0
Original line number Diff line number Diff line 
<testcase>

<info>

<keywords>

IMAP

Clear Text

FETCH

</keywords>

</info>



#

# Server-side

<reply>

<data>

From: me@somewhere

To: fake@nowhere



body



--

  yours sincerely

</data>

<datacheck>

</datacheck>

<servercmd>

</servercmd>

</reply>



#

# Client-side

<client>

<server>

imap

</server>

 <name>

IMAP multi transfer error without curl_multi_remove_handle

 </name>

# tool is what to use instead of 'curl'

<tool>

lib1552

</tool>

 <command>

'imap://%HOSTIP:%IMAPPORT/1552/;UID=1'

</command>

</client>



#

# Verify data after the test has been "shot"

<verify>

<protocol>

</protocol>

</verify>

</testcase>

tests/libtest/Makefile.inc

+4 −1
Original line number Diff line number Diff line
@@ -27,7 +27,7 @@ noinst_PROGRAMS = chkhostname libauthretry libntlmconnect \ 
 lib1525 lib1526 lib1527 lib1528 lib1529 lib1530 lib1531 lib1532 lib1533 \

 lib1534 lib1535 lib1536 lib1537 lib1538 \

 lib1540 \

 lib1550 lib1551 \

 lib1550 lib1551 lib1552 \

 lib1900 \

 lib2033
@@ -454,6 +454,9 @@ lib1550_CPPFLAGS = $(AM_CPPFLAGS) -DLIB1517 
lib1551_SOURCES = lib1551.c $(SUPPORTFILES)

lib1551_CPPFLAGS = $(AM_CPPFLAGS)



lib1552_SOURCES = lib1552.c $(SUPPORTFILES) $(TESTUTIL)

lib1552_CPPFLAGS = $(AM_CPPFLAGS)



lib1900_SOURCES = lib1900.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS)

lib1900_LDADD = $(TESTUTIL_LIBS)

lib1900_CPPFLAGS = $(AM_CPPFLAGS)

tests/libtest/lib1552.c

0 → 100644
+93 −0
Original line number Diff line number Diff line 
/***************************************************************************

 *                                  _   _ ____  _

 *  Project                     ___| | | |  _ \| |

 *                             / __| | | | |_) | |

 *                            | (__| |_| |  _ <| |___

 *                             \___|\___/|_| \_\_____|

 *

 * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.

 *

 * This software is licensed as described in the file COPYING, which

 * you should have received as part of this distribution. The terms

 * are also available at https://curl.haxx.se/docs/copyright.html.

 *

 * You may opt to use, copy, modify, merge, publish, distribute and/or sell

 * copies of the Software, and permit persons to whom the Software is

 * furnished to do so, under the terms of the COPYING file.

 *

 * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY

 * KIND, either express or implied.

 *

 ***************************************************************************/

#include "test.h"



#include "testutil.h"

#include "warnless.h"

#include "memdebug.h"



#define TEST_HANG_TIMEOUT 60 * 1000



int test(char *URL)

{

  CURL *curls = NULL;

  CURLM *multi = NULL;

  int still_running;

  int i = 0;

  int res = 0;

  CURLMsg *msg;

  int counter = 3;



  start_test_timing();



  global_init(CURL_GLOBAL_ALL);



  multi_init(multi);



  easy_init(curls);



  easy_setopt(curls, CURLOPT_URL, URL);

  easy_setopt(curls, CURLOPT_HEADER, 1L);

  easy_setopt(curls, CURLOPT_VERBOSE, 1L);

  easy_setopt(curls, CURLOPT_USERPWD, "u:s");



  multi_add_handle(multi, curls);



  multi_perform(multi, &still_running);



  abort_on_test_timeout();



  while(still_running && counter--) {

    int num;

    res = curl_multi_wait(multi, NULL, 0, TEST_HANG_TIMEOUT, &num);

    if(res != CURLM_OK) {

      printf("curl_multi_wait() returned %d\n", res);

      res = TEST_ERR_MAJOR_BAD;

      goto test_cleanup;

    }



    abort_on_test_timeout();



    multi_perform(multi, &still_running);



    abort_on_test_timeout();

  }



  msg = curl_multi_info_read(multi, &still_running);

  if(msg)

    /* this should now contain a result code from the easy handle,

       get it */

    i = msg->data.result;



test_cleanup:



  /* undocumented cleanup sequence - type UA */



  curl_multi_cleanup(multi);

  curl_easy_cleanup(curls);

  curl_global_cleanup();



  if(res)

    i = res;



  return i; /* return the final return code */

}