MySQL Bugs: #104856: Possible SIGSEGV in set_parse_error

Bug #104856	Possible SIGSEGV in set_parse_error_message
Submitted:	7 Sep 2021 11:54	Modified:	17 Nov 2021 12:56
Reporter:	Dmitriy Philimonov	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Stored Routines	Severity:	S2 (Serious)
Version:	8.0	OS:	Any
Assigned to:		CPU Architecture:	Any

Description:
=== Location === 

In file sql/table_trigger_dispatcher.h

Related commit: https://github.com/mysql/mysql-server/commit/f20f049f0e896fda2479ea5aa3efc1136e6b176e

Function: https://github.com/mysql/mysql-server/blob/8.0/sql/table_trigger_dispatcher.h#L160

```cpp
  void set_parse_error_message(const char *error_message) {
    if (!m_has_unparseable_trigger) {
      m_has_unparseable_trigger = true;
      strncpy(m_parse_error_message, error_message,
              sizeof(m_parse_error_message) - 1);
      m_parse_error_message[sizeof(m_parse_error_message) - 1] = '\n';
    }
  }
```

Could lead to SIGSEGV/undefined behaviour.

=== Description ===

From `man strncpy`:

If there is no terminating null byte in the first n bytes of src, strncpy() produces an unterminated string in dest. If buf has  length  buflen,  you can force termination using something like the following:

           if (buflen > 0) {
               strncpy(buf, str, buflen - 1);
               buf[buflen - 1]= '\0';
           }

The consumer of m_parse_error_message is this function:
  bool check_for_broken_triggers() {
    if (m_has_unparseable_trigger) {
      my_message(ER_PARSE_ERROR, m_parse_error_message, MYF(0));
      return true;
    }
    return false;
  }

`my_message` contains callbacks to:
```shell
init.cc
51:  error_handler_hook = my_message_stderr;

mysqld.cc
7258:  error_handler_hook = my_message_sql;
```

Both functions expect C-string as the input parameter, so with the upstream code it may get SIGSEGV/undefined behaviour in the nearest strlen/printf("...%s...")

How to repeat:
Current build is stable because the size of `m_parse_error_message` buffer is large enough for current set of error messages. However, it's a classical ticking bomb.

Suggested fix:
Ensure C-string is NULL-terminated:
```cpp
  void set_parse_error_message(const char *error_message) {
    if (!m_has_unparseable_trigger) {
      m_has_unparseable_trigger = true;
      strncpy(m_parse_error_message, error_message,
              sizeof(m_parse_error_message) - 1);
      m_parse_error_message[sizeof(m_parse_error_message) - 1] = '\0';
    }
  }
```

Hi Mr. Philimonov,

Thank you for your bug report.

We fully agree with your analysis of the problem.

Verified as reported.

Documented fix as follows in the MySQL 8.0.28 changelog:

    Code inspection showed used of strncpy() in the internal
    function set_parse_error_message() without ensuring that the
    last byte of the buffer being copied into was a null byte. We
    fix this by using snprintf() instead; this makes sure that the
    result is valid even if it is truncated.

Closed.

Thank you, Jon !!!!