obgm / libcoap

A CoAP (RFC 7252) implementation in C
Other
790 stars 422 forks source link

heap buffer overflow read in coap_send function #1063

Closed zambbo closed 1 year ago

zambbo commented 1 year ago

Environment

Problem Description

I'm have been fuzzing libcoap library. And I find a heap-buffer-overflow read bug in coap_send function. In coap_send_internal function in src/net.c
https://github.com/obgm/libcoap/blob/da6459416f32f2bb00301314b138988f1de09566/src/net.c#L1289-L1293 heap buffer overflow read occur in strstr function. When inserting a null character at the end of pdu->data, it is calculated and inserted based on data_len, but since there is no length verification for data_len, there are cases where it exceeds the allowed range of pdu->data.

Expected Behavior

no buffer over read in coap_send function.

Actual Behavior

buffer over read

Steps to reproduce

  1. compile under code with clang -fsanitize=address -I$(header folder location) -L$(library folder location) -lcoap-3 -o $(filename) PoC.c
  2. run and see asan report.

Code to reproduce this issue

// the code should be wrapped in the ```cpp tag so that it will be displayed
better.
#include <coap3/coap.h>

int main() {
 uint8_t data[] = {0x40, 0xa8, 0x38, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x12, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x12, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x97, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a, 0x9a};
 size_t size = sizeof(data);
 coap_pdu_t *pdu = coap_pdu_init(0, 0, 0, size);
 if (!pdu) {
     return 0;
 }

 int ret = coap_pdu_parse(COAP_PROTO_UDP, (uint8_t *)data, size, pdu);
 if (ret <= 0) {
     coap_delete_pdu(pdu);
     return 0;
 }

 coap_address_t dst;
 coap_address_init(&dst);
 dst.addr.sa.sa_family = AF_INET;
 dst.addr.sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
 dst.addr.sin.sin_port = htons(COAP_DEFAULT_PORT);

 coap_context_t *ctx = coap_new_context(NULL);
 if (!ctx) {
     coap_delete_pdu(pdu);
     return 0;
 }

 coap_session_t *session = coap_new_client_session(ctx, NULL, &dst, COAP_PROTO_UDP);
 if (!session) {
     coap_delete_pdu(pdu);
     coap_free_context(ctx);
     return 0;
 }

 coap_send(session, pdu);

 coap_session_release(session);
 coap_free_context(ctx);

 return 0;
}

Debug Logs

=================================================================
==1664==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x612000019f6b at pc 0x556819a1e8ea bp 0x7ffc6c55a920 sp 0x7ffc6c55a0e0
READ of size 293 at 0x612000019f6b thread T0
    #0 0x556819a1e8e9 in StrstrCheck(void*, char*, char const*, char const*) asan_interceptors.cpp.o
    #1 0x556819a1e721 in strstr (/home/zambbo/CoapFuzzer/coap_send_fuzz+0x6f721) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)
    #2 0x7ff697290804 in coap_send_internal /home/libcoap/src/net.c:1368:21
    #3 0x556819ac4ba8 in LLVMFuzzerTestOneInput (/home/zambbo/CoapFuzzer/coap_send_fuzz+0x115ba8) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)
    #4 0x5568199ed423 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/zambbo/CoapFuzzer/coap_send_fuzz+0x3e423) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)
    #5 0x5568199ecb79 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/zambbo/CoapFuzzer/coap_send_fuzz+0x3db79) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)
    #6 0x5568199ee369 in fuzzer::Fuzzer::MutateAndTestOne() (/home/zambbo/CoapFuzzer/coap_send_fuzz+0x3f369) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)
    #7 0x5568199eeee5 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile> >&) (/home/zambbo/CoapFuzzer/coap_send_fuzz+0x3fee5) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)
    #8 0x5568199dd022 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/zambbo/CoapFuzzer/coap_send_fuzz+0x2e022) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)
    #9 0x556819a06d12 in main (/home/zambbo/CoapFuzzer/coap_send_fuzz+0x57d12) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)
    #10 0x7ff696f68d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
    #11 0x7ff696f68e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
    #12 0x5568199d1a64 in _start (/home/zambbo/CoapFuzzer/coap_send_fuzz+0x22a64) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)

0x612000019f6b is located 0 bytes to the right of 299-byte region [0x612000019e40,0x612000019f6b)
allocated by thread T0 here:
    #0 0x556819a89ec6 in __interceptor_realloc (/home/zambbo/CoapFuzzer/coap_send_fuzz+0xdaec6) (BuildId: f509aeba316cf3517cb78dd76034061553078e58)
    #1 0x7ff697295b95 in coap_pdu_resize /home/libcoap/src/pdu.c:264:25

SUMMARY: AddressSanitizer: heap-buffer-overflow asan_interceptors.cpp.o in StrstrCheck(void*, char*, char const*, char const*)
Shadow bytes around the buggy address:
  0x0c247fffb390: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c247fffb3a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c247fffb3b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c247fffb3c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fffb3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c247fffb3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00[03]fa fa
  0x0c247fffb3f0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c247fffb400: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c247fffb410: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
  0x0c247fffb420: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c247fffb430: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
mrdeep1 commented 1 year ago

@zambbo Thank you for raising this. I believe that (adding in if (coap_pdu_resize(pdu, pdu->used_size + 1)) {wrapper)

diff --git a/src/net.c b/src/net.c
index e62eaa67..ac429bc9 100644
--- a/src/net.c
+++ b/src/net.c
@@ -1287,17 +1287,19 @@ coap_send_internal(coap_session_t *session, coap_pdu_t *pdu) {
       /* Need to check that we are not seeing this proxy in the return loop */
       if (pdu->data && opt == NULL) {
         if (pdu->used_size + 1 <= pdu->max_size) {
-          char *a_match;
-          size_t data_len = pdu->used_size - (pdu->data - pdu->token);
-          pdu->data[data_len] = '\000';
-          a_match = strstr((char*)pdu->data, cp);
-          if (a_match && (a_match == (char*)pdu->data || a_match[-1] == ' ') &&
-              ((size_t)(a_match - (char*)pdu->data + len) == data_len ||
-               a_match[len] == ' ')) {
-            coap_log_warn("Proxy loop detected '%s'\n",
-                     (char*)pdu->data);
-            coap_delete_pdu(pdu);
-            return (coap_mid_t)COAP_DROPPED_RESPONSE;
+          if (coap_pdu_resize(pdu, pdu->used_size + 1)) {
+            char *a_match;
+            size_t data_len = pdu->used_size - (pdu->data - pdu->token);
+            pdu->data[data_len] = '\000';
+            a_match = strstr((char*)pdu->data, cp);
+            if (a_match && (a_match == (char*)pdu->data || a_match[-1] == ' ') &&
+                ((size_t)(a_match - (char*)pdu->data + len) == data_len ||
+                 a_match[len] == ' ')) {
+              coap_log_warn("Proxy loop detected '%s'\n",
+                       (char*)pdu->data);
+              coap_delete_pdu(pdu);
+              return (coap_mid_t)COAP_DROPPED_RESPONSE;
+            }
           }
         }
       }

should fix your issue. Please confirm

zambbo commented 1 year ago

After modifying the code, there are no errors occurring.

mrdeep1 commented 1 year ago

Fixed (in slightly re-worked code) in #1065.

tijuca commented 1 year ago

Is there a respective release for 4.3.1+ planned which does include this fix? This issue has got the CVE number CVE-2023-3062 assigned and typically fixed versions will get prepared by the upstream projects to handle such issues. Note that only the CVE fix is targeted by such releases!

I've tried to cherry pick the underlying commit on top of tag v4.3.1 but this is failing as the code has changed afterwards in src/net.c. I'll need help from the upstream project to prepare fixed version for Debian.

BTW: The GitHub project of libcoap could benefit if there would be some information available in case security issues need to get reported. There is no further information available on https://github.com/obgm/libcoap/security

mrdeep1 commented 1 year ago

@tijuca Thanks for raising this - I was not aware of this CVE. The code that fixes this for 4.3.1 is

iff --git a/src/net.c b/src/net.c
index 9885944..e259ab0 100644
--- a/src/net.c
+++ b/src/net.c
@@ -1305,19 +1305,27 @@ coap_send_internal(coap_session_t *session, coap_pdu_t *pdu)

       /* Need to check that we are not seeing this proxy in the return loop */
       if (pdu->data && opt == NULL) {
-        if (pdu->used_size + 1 <= pdu->max_size) {
-          char *a_match;
-          size_t data_len = pdu->used_size - (pdu->data - pdu->token);
-          pdu->data[data_len] = '\000';
-          a_match = strstr((char*)pdu->data, cp);
-          if (a_match && (a_match == (char*)pdu->data || a_match[-1] == ' ') &&
-              ((size_t)(a_match - (char*)pdu->data + len) == data_len ||
-               a_match[len] == ' ')) {
-            coap_log(LOG_WARNING, "Proxy loop detected '%s'\n",
-                     (char*)pdu->data);
-            coap_delete_pdu(pdu);
-            return (coap_mid_t)COAP_DROPPED_RESPONSE;
-          }
+        char *a_match;
+        size_t data_len;
+
+        if (pdu->used_size + 1 > pdu->max_size) {
+          /* No space */
+          return (coap_mid_t)COAP_DROPPED_RESPONSE;
+        }
+        if (!coap_pdu_resize(pdu, pdu->used_size + 1)) {
+          /* Internal error */
+          return (coap_mid_t)COAP_DROPPED_RESPONSE;
+        }
+        data_len = pdu->used_size - (pdu->data - pdu->token);
+        pdu->data[data_len] = '\000';
+        a_match = strstr((char*)pdu->data, cp);
+        if (a_match && (a_match == (char*)pdu->data || a_match[-1] == ' ') &&
+            ((size_t)(a_match - (char*)pdu->data + len) == data_len ||
+             a_match[len] == ' ')) {
+          coap_log(LOG_WARNING, "Proxy loop detected '%s'\n",
+                   (char*)pdu->data);
+          coap_delete_pdu(pdu);
+          return (coap_mid_t)COAP_DROPPED_RESPONSE;
         }
       }
       if (pdu->used_size + len + 1 <= pdu->max_size) {

I see also that CVE-2023-35862 has been published (which I did know was likely to happen) but this is for code post 4.3.1.

It is currently planned to release 4.3.2 when #611 is signed off and merged. However, what is the best way to name an interim release that contains CVE-2023-30362 if I was to do that to help you?

tijuca commented 1 year ago

Thanks for the diff, that's what I was needing. I've prepared a new Debian version 4.3.1-2 which is including the fix for CVE-2023-30362.

Newer usptream versions are targeted for unstable/sid and migrate then to testing. The version in the stable releases of Debian do not change normally. CVE fixes or regressions get fixed by only fixing the underlying problem. Newer versions for the stable release can be provided by the backports archive (which I've done regularly in the past).

The other CVE issue I need to compare with the security team. But I suspect this this get need to be fixed the same as this one here.

mrdeep1 commented 1 year ago

The other CVE issue I need to compare with the security team. But I suspect this this get need to be fixed the same as this one here.

The code in question (OSCORE support) is not in the 4.3.1 release, only in the develop branch.

tijuca commented 1 year ago

Then this doesn't affects the current version in Debian stable. Thanks for clear out.

fpic78 commented 9 months ago

Hi. I have a version libcoap 4.2.1. Is this version affected by this CVE? Function coap_send_internal is not present here. Can you provide a patch if this version is affected? Thanks

mrdeep1 commented 9 months ago

4.2.1 is not affected. This issue was introduced when support for RFC8768 was added (4.3.0).

fpic78 commented 9 months ago

Ok. Thanks for you confirmation on my suspects.