Memory leak at handle_request

[zyingp]

The leak can be reproduced as follows:

1. Build the project with ASan enabled and with assert() disabled:
   $ CXXFLAGS="-DNDEBUG=1" CFLAGS="-g -O0 -fsanitize=address -fno-omit-frame-pointer -DNDEBUG=1" LDFLAGS="-g -O0 -fsanitize=address -fno-omit-frame-pointer -DNDEBUG=1" ./configure --disable-tests --disable-documentation --enable-examples --disable-dtls --disable-shared

2. Download a network sample input from https://github.com/zyingp/temp/raw/master/libcoap/src_net_abort Start the coap-server from a terminal:
   $ ./examples/coap-server
   In another terminal, switch to the folder has src_net_abort, and run:
   $ nc 127.0.0.1 5683 < src_net_abort
   Hit Ctrl+C in the first terminal, and you should see output like below

$ ./examples/coap-server
Aug 07 16:48:53:642799 DEBG created UDP endpoint [::]:5683
Aug 07 16:48:53:644229 DEBG created TCP endpoint [::]:5683
Aug 07 16:48:56:906199 DEBG ***[::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : new incoming session
Aug 07 16:48:56:907329 DEBG ***EVENT: 0x1001
Aug 07 16:48:56:907795 DEBG ***[::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : sending CSM
Aug 07 16:48:56:908294 DEBG * [::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : sent 6 bytes
v:1 t:CON c:CSM i:0000 {} [ Max-Message-Size:8388864 ]
Aug 07 16:48:56:909039 DEBG * [::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : received 1443 bytes
v:1 t:CON c:CSM i:0000 {} [ 1:, Max-Message-Size:8 ]
Aug 07 16:48:56:909685 DEBG ***[::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : session connected
Aug 07 16:48:56:910067 DEBG ***EVENT: 0x2001
Aug 07 16:48:56:910455 DEBG coap_pdu_parse: invalid Token
v:1 t:CON c:2.15 i:0000 {} [ ]
Aug 07 16:48:56:910954 DEBG invalid option length
Aug 07 16:48:56:911255 DEBG coap_pdu_parse: missing payload start code
v:1 t:CON c:7.31 i:0000 {ff} [ ]
v:1 t:CON c:7.31 i:0000 {01} [ ]
v:1 t:CON c:GET i:0000 {} [ ]
Aug 07 16:48:56:911754 DEBG call custom handler for resource ''
Aug 07 16:48:56:912049 WARN coap_pdu_resize: pdu too big
Aug 07 16:48:56:912304 WARN coap_pdu_resize: pdu too big
Aug 07 16:48:56:912598 DEBG * [::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : sent 8 bytes
v:1 t:ACK c:2.05 i:0000 {} [ ETag:\x1A\xA7\xE9\x26, Content-Format:text/plain ]
v:1 t:CON c:GET i:0000 {21} [ ]
Aug 07 16:48:56:912953 DEBG call custom handler for resource ''
Aug 07 16:48:56:913215 WARN coap_pdu_resize: pdu too big
Aug 07 16:48:56:913457 WARN coap_pdu_resize: pdu too big
Aug 07 16:48:56:913726 DEBG * [::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : sent 8 bytes
v:1 t:ACK c:2.05 i:0000 {21} [ ETag:\x1A\xA7\xE9\x26 ]
v:1 t:CON c:GET i:0000 {ff} [ ]
Aug 07 16:48:56:914065 DEBG call custom handler for resource ''
Aug 07 16:48:56:914316 WARN coap_pdu_resize: pdu too big
Aug 07 16:48:56:914556 WARN coap_pdu_resize: pdu too big
Aug 07 16:48:56:914819 DEBG * [::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : sent 8 bytes
v:1 t:ACK c:2.05 i:0000 {ff} [ ETag:\x1A\xA7\xE9\x26 ]
v:1 t:CON c:1.00 i:0000 {01} [ ]
Aug 07 16:48:56:915140 DEBG dropped message with invalid code (1.00)
Aug 07 16:48:56:915417 DEBG * [::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : sent 2 bytes
v:1 t:RST c:0.00 i:0000 {} [ ]
v:1 t:CON c:GET i:0000 {0101100120011b80} [ ]
Aug 07 16:48:56:921329 DEBG call custom handler for resource ''
Aug 07 16:48:56:921827 WARN cannot generate response
Aug 07 16:48:56:922238 DEBG coap_pdu_parse: invalid Token
Aug 07 16:48:58:295117 DEBG ***[::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : session disconnected (reason 1)
Aug 07 16:48:58:297167 DEBG ***EVENT: 0x1002
Aug 07 16:48:58:297577 DEBG ***EVENT: 0x2002
^CAug 07 16:49:00:587410 DEBG select: Interrupted system call (4)
Aug 07 16:49:00:588047 DEBG ***[::ffff:127.0.0.1]:5683 <-> [::ffff:127.0.0.1]:4947 TCP : session closed

=================================================================
==21034==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 56 byte(s) in 1 object(s) allocated from:
#0 0x7f862d11eb40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f862e42fa7d in coap_malloc_type src/mem.c:28
#2 0x7f862e44243f in coap_pdu_init src/pdu.c:82
#3 0x7f862e43c416 in handle_request src/net.c:2208
#4 0x7f862e43e61e in coap_dispatch src/net.c:2506
#5 0x7f862e4370c9 in coap_read_session src/net.c:1273
#6 0x7f862e438856 in coap_io_do_events src/net.c:1520
#7 0x7f862e41ffb9 in coap_run_once src/coap_io.c:1587
#8 0x7f862e411648 in main /mnt/d/zyp/fuzzer/fuzzed_projects/libcoap/libcoap-4.2.1/examples/coap-server.c:1023
#9 0x7f862cc61b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

Indirect leak of 12 byte(s) in 1 object(s) allocated from:
#0 0x7f862d11eb40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f862e42fa7d in coap_malloc_type src/mem.c:28
#2 0x7f862e4424f9 in coap_pdu_init src/pdu.c:104
#3 0x7f862e43c416 in handle_request src/net.c:2208
#4 0x7f862e43e61e in coap_dispatch src/net.c:2506
#5 0x7f862e4370c9 in coap_read_session src/net.c:1273
#6 0x7f862e438856 in coap_io_do_events src/net.c:1520
#7 0x7f862e41ffb9 in coap_run_once src/coap_io.c:1587
#8 0x7f862e411648 in main /mnt/d/zyp/fuzzer/fuzzed_projects/libcoap/libcoap-4.2.1/examples/coap-server.c:1023
#9 0x7f862cc61b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

SUMMARY: AddressSanitizer: 68 byte(s) leaked in 2 allocation(s).

(Sending the sample network input again, the leaked memory size will double.)

[zyingp]

(Some further information.)

1. The leak reason is that one of the PDU will fail in the coap_add_token() method in handle_request(), which causes the control to go to the else clause.

   /* Implementation detail: coap_add_token() immediately returns 0
   if response == NULL */
   if (coap_add_token(response, pdu->token_length, pdu->token)) {

However, in the leak clause, a coap_delete_pdu(response); clause is missing.

} else {
  coap_log(LOG_WARNING, "cannot generate response\r\n");
}

My test is based on the latest version 4.2.1, but the same problem should exist in the master branch as well.

2. Below is the snapshot of my gdb debugging:

...
Run till exit from #0 coap_pdu_init (type=2 '\002', code=0 '\000', tid=0, size=6) at src/pdu.c:83
0x000000000803c417 in handle_request (context=0x611000000040, session=0x613000000200,
pdu=0x606000000680) at src/net.c:2208
2208 response = coap_pdu_init(pdu->type == COAP_MESSAGE_CON
Value returned is $4 = (coap_pdu_t *) 0x6060000006e0
(gdb) n
2215 if (coap_add_token(response, pdu->token_length, pdu->token)) {
(gdb)
2287 coap_log(LOG_WARNING, "cannot generate response\r\n");
(gdb)
Aug 07 17:16:15:406469 WARN cannot generate response
2311 coap_delete_string(uri_path);
(gdb)
2087 handle_request(coap_context_t *context, coap_session_t *session, coap_pdu_t *pdu) {
(gdb)

3. Possible fix?
   Adding coap_delete_pdu(response); in the else clause.

   } else {
   coap_log(LOG_WARNING, "cannot generate response\r\n");
   }

FYI: I find the leak by my fuzzing tool MultiFuzz.

[mrdeep1]

@zyingp Thanks for your information provided and reporting/debugging of this issue.

coap_add_token() can fail in 4 different ways - 3 of them if the PDU is allocated - in this case because a previous bad input PDU with the TCP' CSM response defining the Max-Message-Size as 8 and then this size restricted a subsequent response that needed more space (token size of 8 and need for option end marker).

In all 3 error cases, the PDU does need to be de-allocated.

A fix would be

diff --git a/src/net.c b/src/net.c
index ecdd101..d2623a3 100644
--- a/src/net.c
+++ b/src/net.c
@@ -2366,6 +2366,10 @@ handle_request(coap_context_t *context, coap_session_t *s
       response = NULL;
     } else {
       coap_log(LOG_WARNING, "cannot generate response\r\n");
+      if (response) {
+        coap_delete_pdu(response);
+        response = NULL;
+      }
     }
   } else {
     if (coap_string_equal(uri_path, &coap_default_uri_wellknown)) {

I will raise a PR for it.