MySQL Bugs: #39591: Crash if table comment is longer than 62 characters

Bug #39591	Crash if table comment is longer than 62 characters
Submitted:	22 Sep 2008 16:06	Modified:	22 Nov 2008 9:07
Reporter:	Mattias Jonsson	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: DDL	Severity:	S1 (Critical)
Version:	6.0	OS:	Any
Assigned to:	Tatiana Azundris Nuernberg	CPU Architecture:	Any

Description:
If building 6.0 with BUILD/compile-pentium-debug-max it crashes if one uses a TABLE COMMENT longer than 62 characters
(Tested on latest bzr main, and also on the released 6.0.6-alpha).

It does not crash on the distributed binary 6.0.6-alpha (using the osx 10.5 tar.gz release)

I have not succeeded to repeat it in 5.1

How to repeat:
CREATE TABLE t1 (a INT) COMMENT 'This is a comment of 62 chars 12345678901234567890123456789012';

Thank you for the bug report. This bug probably was introduced in the last pull since a Windows server 2 days older don't crash.

Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 1
Server version: 6.0.8-alpha-debug Source distribution

Type 'help;' or '\h' for help. Type '\c' to clear the buffer.

mysql> CREATE TABLE t1 (a INT) COMMENT 'This is a comment of 62 chars
    '> 12345678901234567890123456789012';
ERROR 2013 (HY000): Lost connection to MySQL server during query
mysql> 

c:\dbs>c:\dbs\6.0\bin\mysql -uroot --port=3600 --prompt="mysql 6.0 > "
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 2
Server version: 6.0.8-alpha-nt-debug-log Source distribution

Type 'help;' or '\h' for help. Type '\c' to clear the buffer.

mysql 6.0 > use test
Database changed
mysql 6.0 > CREATE TABLE t1 (a INT) COMMENT 'This is a comment of 62 chars
    '> 12345678901234567890123456789012';
Query OK, 0 rows affected (0.05 sec)

mysql 6.0 >

Crashes here:
    forminfo[46]=(uchar) create_info->comment.length;
#ifdef EXTRA_DEBUG
    /*
      EXTRA_DEBUG causes strmake() to initialize its buffer behind the
      payload with a magic value to detect wrong buffer-sizes. We
      explicitly zero that segment again.
    */
    memset((char*) forminfo+47 + forminfo[46], 0, 61 - forminfo[46]);
#endif

I don't know who added this, bzr gannotate wouldn't show, my gut feeling is that it's Monty.

Guilhem Bichot writes:

The code where crash happens (EXTRA_DEBUG etc) comes from Dec 2007 by:

sp1r-tnurnberg@mysql.com/white.intern.koehntopp.de-20071206104827-11862

with comment:
Bug#31752: check strmake() bounds
post-fixes: prevent semi-related overflow, additional comments

I saw this by looking at gannotate for the file in 5.1 branch. When I look
into 6.0, gannotate shows another revision (from Alik), which is wrong (I
filed https://bugs.launchpad.net/bzr/+bug/277537 and a support incident at
Canonical), so I understand when you write "gannotate wouldn't show".

Also note, Miguel wrote on Sep 22 "This bug probably was introduced in the
last pull since a Windows server 2 days older don't crash", but the
EXTRA_DEBUG thing is Dec 2007.

Tatjana, please fix ASAP :)

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/56856

2886 Tatiana A. Nurnberg	2008-10-23
      Bug#39591 - Crash if table comment is longer than 62 characters
      
      Maximum length for TABLE..COMMENT changed between 5.1 and 6.0; code adjusted.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/57746

2886 Tatiana A. Nurnberg	2008-11-04
      Bug#39591 - Crash if table comment is longer than 62 characters
      
      Maximum length for TABLE..COMMENT changed between 5.1 and 6.0; code adjusted.

pushed to 6.0.8-bugteam

Pushed into 6.0.8-alpha  (revid:azundris@mysql.com-20081104081049-pw2innwgkvq5lk8k) (version source revid:mattias.jonsson@sun.com-20081104113554-s9cs5edr161qjg78) (pib:5)

Hi Paul,

It's characters, not bytes.

  tmp_len= system_charset_info->cset->charpos(system_charset_info,
                                              create_info->comment.str,
                                              create_info->comment.str +
                                              create_info->comment.length, 
                                              TABLE_COMMENT_MAXLEN);

gives us the byte-position of the character at (character-position, not byte-position) TABLE_COMMENT_MAXLEN. The trick here is that character-positions start at 0, so the last character in a maximum-allowed length string would be at char-pos MAXLEN-1; charpos MAXLEN will be the position of the terminator. Consequently, bytepos(charpos(MAXLEN)) should be equal to comment[length] (which should also be the terminator, or at least the first byte after the payload in the strict sense). If this is not so (bytepos(charpos(MAXLEN)) comes /before/ the end of the string), the string is too long.

  if (tmp_len < create_info->comment.length)
  {
    ... throw warning etc. ...

If the byte-position of the last allowed *character* (not byte) is lower than the position of the string-terminator, the string is too long.

Which means that the commenter possibly hadn't "set names ..." or some such?

Noted in 6.0.8 changelog.

Creating a table with a comment of 62 characters or longer caused a
server crash.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/59390

2719 Tatiana A. Nurnberg	2008-11-20
      Bug#39591: Crash if table comment is longer than 62 characters
      
      It was possible to crash a mysqld build with EXTRA_DEBUG using
      CREATE TABLE ... COMMENT with a specially-crafted UTF-8 string.
      
      This CS removes the check that caused it since it no longer
      applies in current servers anyway, and adds comments instead
      to avoid future confusion.

re-opened bug, attached new CS, because it is my considered opinion that
a) this is the better fix
b) we sorely need more comments
c) this needs to be fixed in 5.1 and 6.0 at least; preferably in 5.0 as well.
   attached CS is for 5.0 and uses numeric literals; CS for 6.0 will again
   use #defined constants and have add'l test case for 4-byte UTF-8.

Pushed into 5.0.76 (revid:joro@sun.com-20090105160414-8q9j4bi1klkfwiup) (version source revid:azundris@mysql.com-20081120143939-2ra1c8iuslx17j0v) (merge vers: 5.0.74) (pib:6)

Pushed into 5.1.31 (revid:joro@sun.com-20090115053147-tx1oapthnzgvs1ro) (version source revid:azundris@mysql.com-20081201041752-ohchvqmhr89joj03) (merge vers: 5.1.31) (pib:6)

Pushed into 5.1.31-ndb-6.2.17 (revid:tomas.ulin@sun.com-20090119095303-uwwvxiibtr38djii) (version source revid:tomas.ulin@sun.com-20090115073240-1wanl85vlvw2she1) (merge vers: 5.1.31-ndb-6.2.17) (pib:6)

Pushed into 5.1.31-ndb-6.3.21 (revid:tomas.ulin@sun.com-20090119104956-guxz190n2kh31fxl) (version source revid:tomas.ulin@sun.com-20090119104956-guxz190n2kh31fxl) (merge vers: 5.1.31-ndb-6.3.21) (pib:6)

Pushed into 5.1.31-ndb-6.4.1 (revid:tomas.ulin@sun.com-20090119144033-4aylstx5czzz88i5) (version source revid:tomas.ulin@sun.com-20090119144033-4aylstx5czzz88i5) (merge vers: 5.1.31-ndb-6.4.1) (pib:6)

Pushed into 6.0.10-alpha (revid:joro@sun.com-20090119171328-2hemf2ndc1dxl0et) (version source revid:azundris@mysql.com-20081201061716-vaiutmfxk7d3mvb6) (merge vers: 6.0.9-alpha) (pib:6)

Noted in 5.0.74sp1, 5.0.76, 5.1.31 changelogs.