Bug #38662 Possible memleak in BaseString::assign

• Submitted: 8 Aug 2008 9:31
• Modified: 21 Nov 2008 12:34
• Reporter: Magnus Blåudd
• Status: Closed
• Category: MySQL Cluster: Cluster (NDB) storage engine
• Severity: S2 (Serious)
• Version: 6.2
• OS: Any
• Assigned to: Magnus Blåudd
• CPU Architecture: Any

[8 Aug 2008 9:31] Magnus Blåudd

Description:
BaseString&
BaseString::assign(const char* s)
{
    if (s == NULL)
    {
      m_chr = NULL; <<< will leak if m_chr != NULL
      m_len = 0;
      return *this;
    }

How to repeat:
MCI

Suggested fix:
Free the memory that m_chr is pointing too.

[24 Oct 2008 14:31] 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/57018

2708 Magnus Svensson	2008-10-24
      Bug#38662 Possible memleak in BasesString::assign
       - Fix the meory leak + two other instances found
       - Update the test programs for BaseString to be built and run automatically by "make test-unit"

[13 Nov 2008 7:33] Jonas Oreland

comment:
1) shouldnt you check if m_chr is null before the delete[]

[20 Nov 2008 15:40] Bugs System

Pushed into 5.1.29-ndb-6.2.17  (revid:[e-mail]-20081120153759-8v4jl6frptxtrzec) (version source revid:[e-mail]-20081120153759-8v4jl6frptxtrzec) (pib:5)

[20 Nov 2008 16:45] Bugs System

Pushed into 5.1.29-ndb-6.3.19  (revid:[e-mail]-20081120153759-8v4jl6frptxtrzec) (version source revid:[e-mail]-20081120162030-yaenl8feqfa36rsw) (pib:5)

[20 Nov 2008 16:49] Bugs System

Pushed into 5.1.29-ndb-6.4.0  (revid:[e-mail]-20081120153759-8v4jl6frptxtrzec) (version source revid:[e-mail]-20081120164218-ark7pv4s1ndaj4mt) (pib:5)

[20 Nov 2008 16:58] Magnus Blåudd

=== modified file 'storage/ndb/src/common/util/BaseString.cpp'
--- storage/ndb/src/common/util/BaseString.cpp	2008-11-20 14:26:32 +0000
+++ storage/ndb/src/common/util/BaseString.cpp	2008-11-20 15:37:59 +0000
@@ -37,6 +37,7 @@
     {
       m_chr = NULL;
       m_len = 0;
+      return;
     }
     const size_t n = strlen(s);
     m_chr = new char[n + 1];
@@ -83,6 +84,8 @@
 {
     if (s == NULL)
     {
+      if (m_chr)
+        delete[] m_chr;
       m_chr = NULL;
       m_len = 0;
       return *this;
@@ -135,6 +138,9 @@
 BaseString&
 BaseString::append(const char* s)
 {
+    if (s == NULL)
+      return *this;
+
     size_t n = strlen(s);
     char* t = new char[m_len + n + 1];
     if (t)
@@ -438,11 +444,13 @@
 char*
 BaseString::trim(char * str, const char * delim){
     int len = strlen(str) - 1;
-    for(; len > 0 && strchr(delim, str[len]); len--);
-    
+    for(; len > 0 && strchr(delim, str[len]); len--)
+      ;
+
     int pos = 0;
-    for(; pos <= len && strchr(delim, str[pos]); pos++);
-    
+    for(; pos <= len && strchr(delim, str[pos]); pos++)
+      ;
+
     if(pos > len){
 	str[0] = 0;
 	return 0;
@@ -539,6 +547,22 @@
 	assert(BaseString("abc\t\n\r kalleabc\t\r\n").trim("abc\t\r\n ") == "kalle");
 	assert(BaseString(" ").trim(" ") == "");
     }
+
+    // Tests for BUG#38662
+    BaseString s2(NULL);
+    BaseString s3;
+    BaseString s4("elf");
+
+    assert(s3.append((const char*)NULL) == "");
+    assert(s4.append((const char*)NULL) == "elf");
+    assert(s4.append(s3) == "elf");
+    assert(s4.append(s2) == "elf");
+    assert(s4.append(s4) == "elfelf");
+
+    assert(s3.assign((const char*)NULL).c_str() == NULL);
+    assert(s4.assign((const char*)NULL).c_str() == NULL);
+    assert(s4.assign(s4).c_str() == NULL);
+
     return 0;
 }

[20 Nov 2008 16:59] Magnus Blåudd

Pushed to 6.2, 6.3 and 6.4

[21 Nov 2008 12:34] 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 bug fix. More information about accessing the source trees is available at

    http://dev.mysql.com/doc/en/installing-source.html

[12 Dec 2008 23:27] Bugs System

Pushed into 6.0.9-alpha  (revid:[e-mail]-20081120153759-8v4jl6frptxtrzec) (version source revid:[e-mail]-20081209185954-9svcixh2p5hsfi6w) (pib:5)