Bug #16347343 : CRASH, GROUP_CONCAT, DERIVED TABLES

      
Problem:
A select query inside a group_concat function having an 
outer reference results in a crash.
      
Analysis:
In function Item_group_concat::add, we do not check if 
return value of get_tmp_table_field can be NULL for 
a non-const item. This can happen for a query with a 
outer reference.
While resolving the outer reference in the query present
inside group_concat function, we set the "const_item_cache" 
to false. As a result in the call to const_item() from 
Item_func_group_concat::add, it returns false and goes on 
to check if this can be NULL resulting in the crash.
get_tmp_table_field does not return NULL for Items of type 
Item_field, Item_result_field and Item_ref. 
For all other items, it returns NULL. 
     
Solution:
Check for the return value of get_tmp_table_field before we 
access field contents.

sql/item_sum.cc:
  Check for the return value of get_tmp_table_field before accessing

1 files changed, 50 insertions, 38 deletions

diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index a0317b49cf5..fd35ab027e1 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -1,4 +1,5 @@
-/* Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2000, 2013  Oracle and/or its affiliates. All
+   rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -2799,9 +2800,9 @@ int group_concat_key_cmp_with_distinct(void* arg, const void* key1,
   for (uint i= 0; i < item_func->arg_count_field; i++)
   {
     Item *item= item_func->args[i];
-    /* 
-      If field_item is a const item then either get_tp_table_field returns 0
-      or it is an item over a const table. 
+    /*
+      If item is a const item then either get_tmp_table_field returns 0
+      or it is an item over a const table.
     */
     if (item->const_item())
       continue;
@@ -2811,9 +2812,13 @@ int group_concat_key_cmp_with_distinct(void* arg, const void* key1,
       the temporary table, not the original field
     */
     Field *field= item->get_tmp_table_field();
-    int res;
+
+    if (!field)
+      continue;
+
     uint offset= field->offset(field->table->record[0])-table->s->null_bytes;
-    if((res= field->cmp((uchar*)key1 + offset, (uchar*)key2 + offset)))
+    int res= field->cmp((uchar*)key1 + offset, (uchar*)key2 + offset);
+    if (res)
       return res;
   }
   return 0;
@@ -2837,23 +2842,25 @@ int group_concat_key_cmp_with_order(void* arg, const void* key1,
   {
     Item *item= *(*order_item)->item;
     /*
+      If item is a const item then either get_tmp_table_field returns 0
+      or it is an item over a const table.
+    */
+    if (item->const_item())
+      continue;
+    /*
       We have to use get_tmp_table_field() instead of
       real_item()->get_tmp_table_field() because we want the field in
       the temporary table, not the original field
-    */
+     */
     Field *field= item->get_tmp_table_field();
-    /* 
-      If item is a const item then either get_tp_table_field returns 0
-      or it is an item over a const table. 
-    */
-    if (field && !item->const_item())
-    {
-      int res;
-      uint offset= (field->offset(field->table->record[0]) -
-                    table->s->null_bytes);
-      if ((res= field->cmp((uchar*)key1 + offset, (uchar*)key2 + offset)))
-        return (*order_item)->asc ? res : -res;
-    }
+    if (!field)
+      continue;
+
+    uint offset= (field->offset(field->table->record[0]) -
+                  table->s->null_bytes);
+    int res= field->cmp((uchar*)key1 + offset, (uchar*)key2 + offset);
+    if (res)
+      return (*order_item)->asc ? res : -res;
   }
   /*
     We can't return 0 because in that case the tree class would remove this
@@ -2889,23 +2896,28 @@ int dump_leaf_key(uchar* key, element_count count __attribute__((unused)),
   for (; arg < arg_end; arg++)
   {
     String *res;
-    if (! (*arg)->const_item())
+    /*
+      We have to use get_tmp_table_field() instead of
+      real_item()->get_tmp_table_field() because we want the field in
+      the temporary table, not the original field
+      We also can't use table->field array to access the fields
+      because it contains both order and arg list fields.
+     */
+    if ((*arg)->const_item())
+      res= (*arg)->val_str(&tmp);
+    else
     {
-      /*
-	We have to use get_tmp_table_field() instead of
-	real_item()->get_tmp_table_field() because we want the field in
-	the temporary table, not the original field
-        We also can't use table->field array to access the fields
-        because it contains both order and arg list fields.
-      */
       Field *field= (*arg)->get_tmp_table_field();
-      uint offset= (field->offset(field->table->record[0]) -
-                    table->s->null_bytes);
-      DBUG_ASSERT(offset < table->s->reclength);
-      res= field->val_str(&tmp, key + offset);
+      if (field)
+      {
+        uint offset= (field->offset(field->table->record[0]) -
+                      table->s->null_bytes);
+        DBUG_ASSERT(offset < table->s->reclength);
+        res= field->val_str(&tmp, key + offset);
+      }
+      else
+        res= (*arg)->val_str(&tmp);
     }
-    else
-      res= (*arg)->val_str(&tmp);
     if (res)
       result->append(*res);
   }
@@ -3138,12 +3150,12 @@ bool Item_func_group_concat::add()
   for (uint i= 0; i < arg_count_field; i++)
   {
     Item *show_item= args[i];
-    if (!show_item->const_item())
-    {
-      Field *f= show_item->get_tmp_table_field();
-      if (f->is_null_in_record((const uchar*) table->record[0]))
+    if (show_item->const_item())
+      continue;
+
+    Field *field= show_item->get_tmp_table_field();
+    if (field && field->is_null_in_record((const uchar*) table->record[0]))
         return 0;                               // Skip row if it contains null
-    }
   }
 
   null_value= FALSE;