Bug #56492 a flaw in String::chop()

• Submitted: 2 Sep 2010 10:15
• Modified: 20 Nov 2010 18:06
• Reporter: Andrei Elkin
• Status: Closed
• Category: MySQL Server
• Severity: S3 (Non-critical)
• OS: Any
• Assigned to: Andrei Elkin
• CPU Architecture: Any
• Tags: string

[2 Sep 2010 10:15] Andrei Elkin

Description:
String::chop() uses postfix decrement inside a subscript operator

  Ptr[str_length--]= '\0'; 

with intent to subtract 1 from the length and put \0 into the indexed by the *new* value of `str_length' position.
While the first part succeeds - the length is changed - the 2nd does not and \0 remains (if it was set there before) at the same position (old `str_length') after the method as it was before the invocation.

While it looks pretty dangerous there are just two invocations for the method in sources that luckily are not affected by the flaw.

Also notice, fixes of the bug are necessary in particular for P_S.SLAVE_STATUS wl#3656 implementation.

How to repeat:
see the sources.

Suggested fix:
--- sql/sql_string.h	2010-04-13 21:56:19 +0000
+++ sql/sql_string.h	2010-09-02 08:32:41 +0000
@@ -196,7 +196,9 @@ public:
   */
   inline void chop()
   {
-    Ptr[str_length--]= '\0'; 
+    str_length--;
+    Ptr[str_length]= '\0';
+    DBUG_ASSERT(strlen(Ptr) == str_length);
   }
 
   inline void free()

[1 Oct 2010 10:14] 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/119616

3310 Andrei Elkin	2010-10-01
      Bug #56492  	a flaw in String::chop()
      
      String::chop() used postfix decrement inside a subscript operator
      
        Ptr[str_length--]= '\0'; 
      
      with intent to subtract 1 from the length and put \0 into the indexed by the *new* value
      of `str_length' position.
      While the first part succeeds - the length is changed - the 2nd does not and \0 remains
      (if it was set there before) at the same position (old `str_length') after the method as
      it was before the invocation.
      
      Logics of chop() corrected and simplified, an assert is added to
      confirm the correct execution ever.
     @ sql/sql_string.h
        logics of chop() corrected and simplified, an assert is added to
        confirm the correct execution ever.

[5 Oct 2010 13:23] Andrei Elkin

Pushed to mysql-next-mr-bugfixing.

[3 Nov 2010 11:04] Bugs System

Pushed into mysql-next-mr (revid:[e-mail]-20101103110337-y38p58l2if9wjrtx) (version source revid:[e-mail]-20101103110337-y38p58l2if9wjrtx) (pib:21)

[13 Nov 2010 16:14] Bugs System

Pushed into mysql-trunk 5.6.99-m5 (revid:[e-mail]-20101113155825-czmva9kg4n31anmu) (version source revid:[e-mail]-20100629074804-359l9m9gniauxr94) (merge vers: 5.6.99-m4) (pib:21)

[20 Nov 2010 18:06] Paul DuBois

Internal change. No user impact. No changelog entry needed.

[30 Jan 2015 14:03] MySQL Verification Team

your DBUG_ASSERT(strlen(Ptr) == str_length);
is not null-friendly ;)