Bug #10840 Improper use of the _cgets() function may lead to visual errors
Submitted: 24 May 2005 19:37 Modified: 23 Jun 2005 2:59
Reporter: Reid Borsuk Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Command-line Clients Severity:S3 (Non-critical)
Version:4.1.12 OS:Microsoft Windows (Windows)
Assigned to: Jim Winstead CPU Architecture:Any

[24 May 2005 19:37] Reid Borsuk
Description:
The MySQL client software incorrectly assumes that a single call to the _cgets() function will empty the waiting input string. When a long string is inputted, multiple calls to _cgets() may be required before all data is given to the program. This currently causes only a minor visual error, as all but the last character (the linefeed) is being read. The prompt will be printed twice, instead of the expected once.

How to repeat:
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 20 to server version: 4.1.11-debug

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

mysql> AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAA
    ->     ->

(Only one -> is expected)

Suggested fix:
Change MySQL.cpp Line 960 from:

      linebuffer[0]= (char) sizeof(linebuffer);
      line= _cgets(linebuffer);

To:

      char tmpbuf[255] = { 0 };
      linebuffer[0]= (char) sizeof(linebuffer)-3;
      do {
            line= _cgets(linebuffer);
            strncat(tmpbuf, line, sizeof(tmpbuf));
      } while (linebuffer[0] == linebuffer[1]);

      tmpbuf[sizeof(tmpbuf)-1] = 0;
      line = (char*) &tmpbuf;
[24 May 2005 19:40] Reid Borsuk
If the buffer is shortened as is recommended in the bug #10841 (and as is demonstrated in the included fix), then user input may be unexpectedly split. This could cause normal SQL to become malformed. Therefore, this bug should be fixed in conjunction with that one.
[8 Jun 2005 0:33] 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/internals/25737
[9 Jun 2005 17:12] 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/internals/25825
[9 Jun 2005 17: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/internals/25826
[11 Jun 2005 6:34] Jim Winstead
Fixed in 4.1.13 and 5.0.8.
[14 Jun 2005 9:09] Timour Katchaounov
I believe that there is an additional problem with the size of 'linebuffer'.
Currently linebuffer in read_lines() is declared as:
  char linebuffer[254];
As a result we want to set linebuffer[0]= 251;
However, (char) 251 is a negative value, and it seems that this is
how _cgets() treats the first byte of linebuffer.

As a result we never exited from the loop
  do { ... } while (linebuffer[0] <= linebuffer[1] + 1);
and the client hangs in this loop forever.

I don't know if this is a problem with my version of the RTL
(I haveVisual C++ 6.0), and what is the expected behavior
of _cgets().

The way I fixed the problem was to change the maximum size of
linebuffer to 127. In this case evrything works as expected.
[17 Jun 2005 17:11] 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/internals/26130
[20 Jun 2005 16:54] 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/internals/26208
[21 Jun 2005 0:30] Jim Winstead
Fixed in 4.1.13 and 5.0.8.
[21 Jun 2005 16:02] Jon Stephens
Thank you for your bug report. This issue has been committed to our
source repository of that product and will be incorporated into the
next release.

If necessary, you can access the source repository and build the latest
available version, including the bugfix, yourself. More information 
about accessing the source trees is available at
    http://www.mysql.com/doc/en/Installing_source_tree.html

Additional info:

Documented in change history for 4.1.13 and 5.0.8. Marked as Closed.
[22 Jun 2005 22:52] Guilhem Bichot
Hi,
Just to mention that Timour's note is very important; I just got "mysql.exe" to crash (using 5.0.7 built from source on Windows with VC++ Express 2005), because indeed in some function called by _cgets(), the sizeInBytes argument was almost 2^32 (i.e. the 254 in linebuffer[0] was interpreted as negative and then internally cast to unsigned int, that is 2^32-254). This caused an assertion failure in the function (failed sizeInBytes <= INT_MAX).