dinhvh / libetpan

Mail Framework for C Language
www.etpan.org
Other
612 stars 284 forks source link

Null Pointer Dereference STATUS Response #420

Closed hackingump closed 1 year ago

hackingump commented 1 year ago

IMAP STATUS Command Null Pointer Dereference

A segmentation fault due to a null pointer dereference has been found in the IMAP STATUS command handling component. The error occurs when mailimap_mailbox_data_status_free in low-level/imap/mailimap_types.c when it tries to free st_info_list of mb_data_status. The segmentation fault is triggered when an invalid STATUS response is received. This can at least lead to a Denial Of Service.

I have attached a ZIP file, which contains files needed for reproduction.

How to reproduce

See reproduction_logs.txt in ZIP on how to reproduce.

Expected output:

dummy@dummy-mars:~/VResearch/$ LD_LIBRARY_PATH=$LD_LIBRARY_PATH:libetpan/src/.libs/ ./sender abc abc
connect: 2
Building structure list
Sending STATUS ..
Segmentation fault (core dumped)

Explanation of Vulnerability

The null pointer dereference occurs when the mailimap_mailbox_data_status structure in mailimap_mailbox_data_status_free is trying to be freed and the info->st_info_list contains a NULL pointer/is NULL.

* STATUS "RUU" ()

When the STATUS response above is received mailimap_mailbox_data_status_parse is called. mailimap_mailbox_data_status_parse will call mailimap_struct_spaced_list_parse. Due to invalid info fields in STATUS response, mailimap_struct_spaced_list_parse will return a MAILIMAP_ERROR_PARSE (code 5). With info fields, I am referring to the information fields that should be included inside the parentheses.

mailimap_mailbox_data_status_parse does not stop execution when a MAILIMAP_ERROR_PARSE is received. Instead, it will continue and call mailimap_mailbox_data_status_new passing down the status_info_list again, which is NULL. See https://github.com/dinhvh/libetpan/blob/master/src/low-level/imap/mailimap_parser.c#L6613 for the MAILIMAP_ERROR_PARSE check.

mailimap_mailbox_data_status_new will then not check whether st_info_list is NULL and add to mb_data_status. Below is the snippet of mailimap_mailbox_data_status_new.

LIBETPAN_EXPORT
struct mailimap_mailbox_data_status *
mailimap_mailbox_data_status_new(char * st_mailbox,
    clist * st_info_list)
{
  struct mailimap_mailbox_data_status * mb_data_status;

  mb_data_status = malloc(sizeof(* mb_data_status));
  if (mb_data_status == NULL)
    return NULL;
  mb_data_status->st_mailbox = st_mailbox;
  // st_info_list is NULL here
  mb_data_status->st_info_list = st_info_list;

  return mb_data_status;
}

Later, when mailimap_mailbox_data_status_free is called to attempt to free the built-up structures, the function does not check whether st_info_list is valid and passes it to clist_foreach. Below are snippets of clist_foreach and clist_begin to illustrate the issue. If a NULL pointer is passed down, this will lead to a segmentation fault.

void clist_foreach(clist * lst, clist_func func, void * data)
{
  clistiter * cur;

  for(cur = clist_begin(lst) ; cur != NULL ; cur = cur->next)
    func(cur->data, data);
}

clistiter * clist_begin(clist * lst) {
  return lst->first;
}

Simplest Fix

The simplest fix I could come up with in a short time period was to check inside mailimap_mailbox_data_status_new whether st_info_list is NULL. If yes, return NULL. See below:

LIBETPAN_EXPORT
struct mailimap_mailbox_data_status *
mailimap_mailbox_data_status_new(char * st_mailbox,
    clist * st_info_list)
{
  struct mailimap_mailbox_data_status * mb_data_status;

  mb_data_status = malloc(sizeof(* mb_data_status));
  // added check whether st_info_list is NULL
  if (mb_data_status == NULL || st_info_list == NULL)
    return NULL;
  mb_data_status->st_mailbox = st_mailbox;
  mb_data_status->st_info_list = st_info_list;

  return mb_data_status;
}

Happy to discuss further and helping in fixing this vulnerability!

PoC-NullPtrDeref-STATUS.zip

mtasaka commented 1 year ago

The above change leaves memleak for mb_data_status, so I think the above change needs fixing. Instead, how about just adding null-check in mailimap_mailbox_data_status_free ?

--- libetpan-1.9.4.orig/src/low-level/imap/mailimap_types.c 2019-11-02 02:58:50.000000000 +0900
+++ libetpan-1.9.4/src/low-level/imap/mailimap_types.c  2022-11-23 14:58:47.075176654 +0900
@@ -1389,9 +1389,12 @@ void
 mailimap_mailbox_data_status_free(struct mailimap_mailbox_data_status * info)
 {
   mailimap_mailbox_free(info->st_mailbox);
-  clist_foreach(info->st_info_list, (clist_func) mailimap_status_info_free,
+  if (info->st_info_list)
+  {
+   clist_foreach(info->st_info_list, (clist_func) mailimap_status_info_free,
         NULL);
-  clist_free(info->st_info_list);
+   clist_free(info->st_info_list);
+  }
   free(info);
 }
utkarsh2102 commented 1 year ago

CVE-2022-4121 has been assigned to this issue. @dinhvh, can you take a look, please? Thanks!

dinhvh commented 1 year ago

Thank you for bringing up this issue!