MySQL Bugs: #33747: Still more cases where passing a nonexistant column name crashes NDBAPI

Bug #33747	Still more cases where passing a nonexistant column name crashes NDBAPI
Submitted:	8 Jan 2008 18:53	Modified:	20 May 2008 10:25
Reporter:	Hartmut Holzgraefe
Status:	Closed
Category:	Server: NDBAPI	Severity:	S2 (Serious)
Version:	5.1.22-ndb-6.3.6	OS:	Any
Assigned to:	Martin Skold	Target Version:	6.0
Triage:	D1 (Critical)

Description:
I just ran into yet another case where passing a nonexistent column (or index) name to a
NDBAPI method crashes the API instead of returning an error code (see also bug #19088,
bug #21036).

This time it is in storage/ndb/src/ndbapi/NdbOperation.cpp:

    300 int
    301 NdbOperation::equal(const char* anAttrName, const char* aValuePassed)
    302 {
    303   return equal_impl(m_accessTable->getColumn(anAttrName), aValuePassed);
    304 }

Passing a nonexistent attribute name here leads to a segfault as getColumn()
returns a NULL pointer and equal_impl() dereferences the pointer passed to it as first
argument without checking.

How to repeat:
See the README file in the attached NDBAPI sample appliaction

Suggested fix:
Check the result of column/index name lookups in convenience wrappers before passing on
the lookup result to other methods expecting a column or index object pointer as argument
instead of passing on the lookup results right away without checking.

A nonexistent column or index name can be the result of a typo in which case it can be
spotted during development, it can also be the result of an ALTER TABLE DROP COLUMN or
DROP INDEX which would break a previously working NDBAPI application in a very obscure
way.

only happens in debug builds this time, the following patch helps on linux
but would break on solaris as there printf("%s", NULL) isn't handled gracefully:

===== storage/ndb/src/ndbapi/NdbOperationSearch.cpp 1.37 vs edited =====
--- 1.37/storage/ndb/src/ndbapi/NdbOperationSearch.cpp  2008-01-09 22:35:51 +01:00
+++ edited/storage/ndb/src/ndbapi/NdbOperationSearch.cpp        2008-01-09 21:41:45
+01:00
@@ -57,7 +57,7 @@
 {
   DBUG_ENTER("NdbOperation::equal_impl");
   DBUG_PRINT("enter", ("col: %s  op: %d  val: 0x%lx",
-                       tAttrInfo->m_name.c_str(), theOperationType,
+                       tAttrInfo ? tAttrInfo->m_name.c_str() : NULL, theOperationType,
                        (long) aValuePassed));
   
   const char* aValue = aValuePassed;

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/45533

ChangeSet@1.2583, 2008-04-17 08:59:16+02:00, mskold@mysql.com +1 -0
  bug#33747 Still more cases where passing a nonexistant column name crashes NDBAPI:
Added checks for existing column

Pushed into 5.1.24-ndb-6.2.14

Pushed into 5.1.24-ndb-6.3.13

Pushed into 5.1.23-ndb-6.4.0

Documented in the 5.1.24-ndb-6.2.16 and 5.1.24-ndb-6.3.14 changelogs as follows:

        Attempting to pass a nonexistent column name to the equal() and
        setValue() methods of NdbOperation caused NDB API applications to crash.
        Now the column name is checked, and an error is returned in the event
        that the column is not found.

Pushed into 6.0.6-alpha 
(revid:sp1r-mskold/marty@mysql.com/quadfish.(none)-20080417065916-25648) (version source
revid:jonas@mysql.com-20080808094047-4e1yiarqa2t3opg3) (pib:5)