Bug #87200 Suspicious UAF at sql/sql_partition.cc

Description:
Hi all,

Our code scanner has reported a use after free at the end of function `generate_partition_syntax`, see steps I commented bellow, would any one have a look and see if there is a real issue?

// ===============================
  if (unlikely(mysql_file_read(fptr, (uchar*)buf, *buf_length, MYF(MY_FNABP))))
  {
    if (!use_sql_alloc)
      my_free(buf);      // <== Step 1: this line frees buf, but haven't assign a NULL to it
    else
      buf= NULL;
  }
  else
    buf[*buf_length]= 0;

close_file:
  if (buf == NULL)           // Step 2: so this line won't know there was an error occurred.
  {
    my_error(ER_INTERNAL_ERROR, MYF(0), "Failed to generate partition syntax");
  }
  mysql_file_close(fptr, MYF(0));
  DBUG_RETURN(buf);         // Step 3: a freed buf is returned to caller
}
// ===============================

Regards,

SourceBrella Inc. 
Alex

How to repeat:
only if `mysql_file_read` in the description block returns error

Suggested fix:
assign NULL to buf variable after some error occurred.

Hi!

This is not a bug, because as you can see in the code of my_free(), pointer is set to NULL upon being freed, there is a check before it, including the magic number, so the code is quite safe.

Hi Sinisa,

to be sure, you are referring the implementation of `my_free` bellow here? I could not find a NULL assignment in this function, and even if there is one, it can't affect the value of `ptr` copied from the caller (they hold a same value, but they are not a same variable) , might still need some help from you.

the `my_free` implementation that I was looking at,
https://github.com/mysql/mysql-server/blob/71f48ab393bce80a59e5a2e498cd1f46f6b43f9a/mysys/...

// =========================================
void my_free(void *ptr)
{
  my_memory_header *mh;

  if (ptr == NULL)
    return;

  mh= USER_TO_HEADER(ptr);
  DBUG_ASSERT(mh->m_magic == MAGIC);
  PSI_MEMORY_CALL(memory_free)(mh->m_key, mh->m_size, mh->m_owner);
  /* Catch double free */
  mh->m_magic= 0xDEAD;
  MEM_FREELIKE_BLOCK(ptr, 0);
  my_raw_free(mh);
}
// =========================================

This seems a true problem. At least, I found two implementations of the function "my_free()", one of which calls "my_raw_free" directly. "my_raw_free" calls "free()" directly. No "null assignment" exists.

The my_free() implementation is here: https://github.com/mysql/mysql-server/blob/5.7/mysys/my_malloc.c#L157

Upon further analysis, this turns out to be a bug, as pointer is not assigned to NULL in one place.

Verified as reported.

Thank you for your contribution.

Hi @ Sinisa

Sure, thank you for your concern

by the way, is a bug like this could be exploitable ?

No, it is not exploitable as the path is quite improbable to force ...