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
Triage: Triaged: D1 (Critical)

[22 Sep 2008 16:06] Mattias Jonsson
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';
[22 Sep 2008 17:05] Miguel Solorzano
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 >
[2 Oct 2008 22:25] Konstantin Osipov
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.
[3 Oct 2008 13:21] Konstantin Osipov
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 :)
[23 Oct 2008 3:02] Bugs System
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.
[4 Nov 2008 8:16] Bugs System
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.
[4 Nov 2008 9:37] Tatiana Azundris Nuernberg
pushed to 6.0.8-bugteam
[10 Nov 2008 10:54] Bugs System
Pushed into 6.0.8-alpha  (revid:azundris@mysql.com-20081104081049-pw2innwgkvq5lk8k) (version source revid:mattias.jonsson@sun.com-20081104113554-s9cs5edr161qjg78) (pib:5)
[19 Nov 2008 13:55] Tatiana Azundris Nuernberg
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.
[19 Nov 2008 13:56] Tatiana Azundris Nuernberg
Which means that the commenter possibly hadn't "set names ..." or some such?
[19 Nov 2008 17:13] Paul Dubois
Noted in 6.0.8 changelog.

Creating a table with a comment of 62 characters or longer caused a
server crash.
[20 Nov 2008 14:45] Bugs System
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.
[20 Nov 2008 14:49] Tatiana Azundris Nuernberg
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.
[6 Jan 2009 13:57] Bugs System
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)
[15 Jan 2009 6:39] Bugs System
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)
[19 Jan 2009 11:32] Bugs System
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)
[19 Jan 2009 13:09] Bugs System
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)
[19 Jan 2009 16:15] Bugs System
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)
[20 Jan 2009 19:00] Bugs System
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)
[9 Jun 2009 19:00] Paul Dubois
Noted in 5.0.74sp1, 5.0.76, 5.1.31 changelogs.