Double free caused by mqtt_doing()

How it was found

mqtt_doing() (lib/mqtt.c) causes a double free under certain conditions. The conditions are (1) an mqtt_send() is unable to send its entire buffer at one time; and (2) the next call to mqtt_send() fails. The bug arises because mqtt_doing() free()s the pointer mq->sendleftovers without nulling it (line 755).

(Source below, from v.8.12.1; the bug appears still present in master branch as of 3/17/2025):

740: static CURLcode mqtt_doing(struct Curl_easy *data, bool *done)
741: {
...
751:   if(mq->nsend) {
752:     /* send the remainder of an outgoing packet */
753:     char *ptr = mq->sendleftovers;
754:     result = mqtt_send(data, mq->sendleftovers, mq->nsend);
755:     free(ptr);
756:     if(result)
757:       return result;
758:   }
...

119: static CURLcode mqtt_send(struct Curl_easy *data,
120:                           char *buf, size_t len)
121: {
122:   CURLcode result = CURLE_OK;
123:   struct MQTT *mq = data->req.p.mqtt;
124:   size_t n;
125:   result = Curl_xfer_send(data, buf, len, FALSE, &n);
126:   if(result)
127:     return result;

729: static CURLcode mqtt_done(struct Curl_easy *data,
730:                           CURLcode status, bool premature)
731: {
732:   struct MQTT *mq = data->req.p.mqtt;
733:   (void)status;
734:   (void)premature;
735:   Curl_safefree(mq->sendleftovers);
736:   Curl_dyn_free(&mq->recvbuf);
737:   return CURLE_OK;
738: }

You can verify the bug using the debugger thusly:

1. Build cURL in VS2022.
2. Set a BP on mqtt_send() line 125, on mqtt_doing() line 754, and on mqtt_done() line 735.
3. Set curl as the default project
4. Set curl properties/debugging/command line arguments to -v mqtt://test.mosquitto.org:1883/test
5. Run curl (F5)
6. When the BP on mqtt_send() fires, step into the call to Curl_xfer_send().
7. Step that function up to the call to Curl_conn_send().
8. Simulate a partial transfer by using the debugger to doctor blen to 2 (its initial value should be 26).
9. Step back out into mqtt_send() and watch it allocate mq->sendleftovers. Note the value of this pointer.
10. Proceed.
11. You'll get a BP in mqtt_doing(). Step into mqtt_send() and simulate an error sending by skipping the call to Curl_xfer_send() and setting result == CURLE_SEND_ERROR.
12. Step back out into mqtt_doing() and watch line 755 free() the pointer you noted in step 9 without nulling it.
13. Proceed.
14. You'll get a BP in mqtt_done() line 735, which will again free() the pointer you noted in step 9. If you built cURL in debug mode, the CRT will throw an exception.

This bug should be reachable in the wild. In Windows builds, the socket is nonblocking (curlx_nonblock() gets called during socket setup) so it should be capable of partial writes [1]. The bug should be reachable if conditions are such that the first call to mqtt_send() sends only a portion of the requested buffer, and the next call to mqtt_send() fails, as by the mqtt server falling offline before the second call.

[1] https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-send says "On nonblocking stream oriented sockets, the number of bytes written can be between 1 and the requested length, depending on buffer availability on both the client and server computers."

Impact

Potentially anything achievable using a double free / use-after-free bug.