Bug#52168 decimal casting catastrophes: crashes and valgrind errors on simple casts

The problem is that if a NULL is stored in an Item_cache_decimal object, the associated my_decimal object is not initialized. However, it is still accessed when val_int() is called. The fix is to check for null_value within val_int(), and return without accessing the my_decimal object when the cached value is NULL. Bug#52122 reports the same issue for val_real(), and this patch also includes fixes for val_real() and val_str() and corresponding test cases from that bug report. Also, NULL is returned from val_decimal() when value is null. This will avoid that callers access an uninitialized my_decimal object. Made similar changes to all other Item_cache classes. Now all val_* methods should return a well defined value when actual value is NULL. mysql-test/r/type_decimal.result: Updated result file with test cases for Bug#52168 and Bug#52122. mysql-test/t/type_decimal.test: Added test cases for Bug#52168 and Bug#52122. sql/item.cc: In Item_cache_*::val_* methods, return a well defined value when actual value is NULL. This is especially important for Item_cache_decimal since otherwise one risk accessing an uninitialized my_decimal object. sql/item.h: Added method Item_cache::has_value() which returns TRUE if cache object contains a non-null value.
2025-07-30 16:24:05 +03:00 · 2010-05-28 17:30:39 +02:00
parent ba36de4f2e
commit 229cc4e191
4 changed files with 96 additions and 18 deletions
--- a/mysql-test/r/type_decimal.result
+++ b/mysql-test/r/type_decimal.result
@ -966,3 +966,31 @@ max(case 1 when 1 then c else null end)
 300.00
 drop table t1;
 End of 5.0 tests
+CREATE TABLE t1 (a INTEGER);
+INSERT INTO t1 VALUES (NULL);
+CREATE TABLE t2 (b INTEGER);
+INSERT INTO t2 VALUES (NULL), (NULL);
+SELECT b FROM t1 JOIN t2 WHERE CONVERT(a, DECIMAL)|CONVERT(b, DECIMAL);
+b
+DROP TABLE t1, t2;
+CREATE TABLE t1 (col0 INTEGER, col1 REAL);
+CREATE TABLE t2 (col0 INTEGER);
+INSERT INTO t1 VALUES (0, 0.0), (NULL, NULL);
+INSERT INTO t2 VALUES (1);
+SELECT 1 FROM t1 
+JOIN
+( 
+SELECT t2.col0 FROM t2 RIGHT JOIN t1 USING(col0) 
+GROUP BY t2.col0
+) AS subq  
+WHERE t1.col1 + CAST(subq.col0 AS DECIMAL);
+1
+SELECT 1 FROM t1 
+JOIN
+( 
+SELECT t2.col0 FROM t2 RIGHT JOIN t1 USING(col0) 
+GROUP BY t2.col0
+) AS subq  
+WHERE CONCAT(t1.col1, CAST(subq.col0 AS DECIMAL));
+1
+DROP TABLE t1, t2;
--- a/mysql-test/t/type_decimal.test
+++ b/mysql-test/t/type_decimal.test
@ -542,3 +542,44 @@ select max(case 1 when 1 then c else null end) from t1 group by c;
 drop table t1;

 --echo End of 5.0 tests
+
+#
+# Bug#52168 decimal casting catastrophes: 
+#           crashes and valgrind errors on simple casts
+#
+
+# Uninitialized read when calling Item_cache_decimal::val_int()
+CREATE TABLE t1 (a INTEGER);
+INSERT INTO t1 VALUES (NULL);
+CREATE TABLE t2 (b INTEGER);
+INSERT INTO t2 VALUES (NULL), (NULL);
+SELECT b FROM t1 JOIN t2 WHERE CONVERT(a, DECIMAL)|CONVERT(b, DECIMAL);
+DROP TABLE t1, t2;
+
+#
+# Bug#52122 crash when converting derived table column to decimal
+#  
+CREATE TABLE t1 (col0 INTEGER, col1 REAL);
+CREATE TABLE t2 (col0 INTEGER);
+INSERT INTO t1 VALUES (0, 0.0), (NULL, NULL);
+INSERT INTO t2 VALUES (1);
+
+# Uninitialized read when calling Item_cache_decimal::val_real()
+SELECT 1 FROM t1 
+JOIN
+( 
+  SELECT t2.col0 FROM t2 RIGHT JOIN t1 USING(col0) 
+  GROUP BY t2.col0
+) AS subq  
+WHERE t1.col1 + CAST(subq.col0 AS DECIMAL);
+
+# Uninitialized read when calling Item_cache_decimal::val_str()
+SELECT 1 FROM t1 
+JOIN
+( 
+  SELECT t2.col0 FROM t2 RIGHT JOIN t1 USING(col0) 
+  GROUP BY t2.col0
+) AS subq  
+WHERE CONCAT(t1.col1, CAST(subq.col0 AS DECIMAL));
+
+DROP TABLE t1, t2;
--- a/sql/item.cc
+++ b/sql/item.cc
@ -7431,7 +7431,7 @@ void Item_cache_int::store(Item *item, longlong val_arg)
 String *Item_cache_int::val_str(String *str)
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return NULL;
  str->set(value, default_charset());
  return str;
@ -7441,7 +7441,7 @@ String *Item_cache_int::val_str(String *str)
 my_decimal *Item_cache_int::val_decimal(my_decimal *decimal_val)
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return NULL;
  int2my_decimal(E_DEC_FATAL_ERROR, value, unsigned_flag, decimal_val);
  return decimal_val;
@ -7450,7 +7450,7 @@ my_decimal *Item_cache_int::val_decimal(my_decimal *decimal_val)
 double Item_cache_int::val_real()
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0.0;
  return (double) value;
 }
@ -7458,7 +7458,7 @@ double Item_cache_int::val_real()
 longlong Item_cache_int::val_int()
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0;
  return value;
 }
@ -7514,7 +7514,7 @@ String *Item_cache_datetime::val_str(String *str)
 my_decimal *Item_cache_datetime::val_decimal(my_decimal *decimal_val)
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value_int())
+  if (!has_value())
    return NULL;
  int2my_decimal(E_DEC_FATAL_ERROR, int_value, unsigned_flag, decimal_val);
  return decimal_val;
@ -7550,7 +7550,7 @@ bool Item_cache_real::cache_value()
 double Item_cache_real::val_real()
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0.0;
  return value;
 }
@ -7558,7 +7558,7 @@ double Item_cache_real::val_real()
 longlong Item_cache_real::val_int()
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0;
  return (longlong) rint(value);
 }
@ -7567,7 +7567,7 @@ longlong Item_cache_real::val_int()
 String* Item_cache_real::val_str(String *str)
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return NULL;
  str->set_real(value, decimals, default_charset());
  return str;
@ -7577,7 +7577,7 @@ String* Item_cache_real::val_str(String *str)
 my_decimal *Item_cache_real::val_decimal(my_decimal *decimal_val)
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return NULL;
  double2my_decimal(E_DEC_FATAL_ERROR, value, decimal_val);
  return decimal_val;
@ -7599,7 +7599,7 @@ double Item_cache_decimal::val_real()
 {
  DBUG_ASSERT(fixed);
  double res;
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0.0;
  my_decimal2double(E_DEC_FATAL_ERROR, &decimal_value, &res);
  return res;
@ -7609,7 +7609,7 @@ longlong Item_cache_decimal::val_int()
 {
  DBUG_ASSERT(fixed);
  longlong res;
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0;
  my_decimal2int(E_DEC_FATAL_ERROR, &decimal_value, unsigned_flag, &res);
  return res;
@ -7618,7 +7618,7 @@ longlong Item_cache_decimal::val_int()
 String* Item_cache_decimal::val_str(String *str)
 {
  DBUG_ASSERT(fixed);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return NULL;
  my_decimal_round(E_DEC_FATAL_ERROR, &decimal_value, decimals, FALSE,
                   &decimal_value);
@ -7629,7 +7629,7 @@ String* Item_cache_decimal::val_str(String *str)
 my_decimal *Item_cache_decimal::val_decimal(my_decimal *val)
 {
  DBUG_ASSERT(fixed);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return NULL;
  return &decimal_value;
 }
@ -7665,7 +7665,7 @@ double Item_cache_str::val_real()
  DBUG_ASSERT(fixed == 1);
  int err_not_used;
  char *end_not_used;
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0.0;
  if (value)
    return my_strntod(value->charset(), (char*) value->ptr(),
@ -7678,7 +7678,7 @@ longlong Item_cache_str::val_int()
 {
  DBUG_ASSERT(fixed == 1);
  int err;
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0;
  if (value)
    return my_strntoll(value->charset(), value->ptr(),
@ -7691,7 +7691,7 @@ longlong Item_cache_str::val_int()
 String* Item_cache_str::val_str(String *str)
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0;
  return value;
 }
@ -7700,7 +7700,7 @@ String* Item_cache_str::val_str(String *str)
 my_decimal *Item_cache_str::val_decimal(my_decimal *decimal_val)
 {
  DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return NULL;
  if (value)
    string2my_decimal(E_DEC_FATAL_ERROR, value, decimal_val);
@ -7712,7 +7712,7 @@ my_decimal *Item_cache_str::val_decimal(my_decimal *decimal_val)

 int Item_cache_str::save_in_field(Field *field, bool no_conversions)
 {
-  if (!value_cached && !cache_value())
+  if (!has_value())
    return 0;
  int res= Item_cache::save_in_field(field, no_conversions);
  return (is_varbinary && field->type() == MYSQL_TYPE_STRING &&
--- a/sql/item.h
+++ b/sql/item.h
@ -3191,6 +3191,15 @@ public:
  {
    return this == item;
  }
+  /**
+     Check if saved item has a non-NULL value.
+     Will cache value of saved item if not already done. 
+     @return TRUE if cached value is non-NULL.
+   */
+  bool has_value()
+  {
+    return (value_cached || cache_value()) && !null_value;
+  }
  virtual void store(Item *item);
  virtual bool cache_value()= 0;
  bool basic_const_item() const