Skip to content

Commit 2e24445

Browse files
author
Tor Didriksen
committed
Bug#14494275 HEAP CORRUPTION IN DECIMAL2BIN WITH LOW MAX_SORT_LENGTH
The problem was that if the user sets a very small max_sort_length this value was honored by the function sortlength() but later ignored by make_sortkey(). This can result in buffer overrun. Solution one: always stay within the bounds of max_sort_length as specified by the user. This is what we have always done, even if the documentation says it is only done for strings/blobs. Sorting results may be a bit strange, since we use only the first few bytes of int/float/decimal/time.
1 parent 96e45ff commit 2e24445

File tree

4 files changed

+229
-51
lines changed

4 files changed

+229
-51
lines changed

‎sql/filesort.cc‎

Lines changed: 62 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ static ha_rows find_all_keys(Sort_param *param,SQL_SELECT *select,
5555
ha_rows *found_rows);
5656
static int write_keys(Sort_param *param, Filesort_info *fs_info,
5757
uint count, IO_CACHE *buffer_file, IO_CACHE *tempfile);
58-
static void make_sortkey(Sort_param *param,uchar *to, uchar *ref_pos);
5958
static void register_used_fields(Sort_param *param);
6059
static int merge_index(Sort_param *param,uchar *sort_buffer,
6160
BUFFPEK *buffpek,
@@ -64,8 +63,6 @@ static int merge_index(Sort_param *param,uchar *sort_buffer,
6463
static bool save_index(Sort_param *param, uint count,
6564
Filesort_info *table_sort);
6665
static uint suffix_length(ulong string_length);
67-
static uint sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length,
68-
bool *multi_byte_charset);
6966
static SORT_ADDON_FIELD *get_addon_fields(ulong max_length_for_sort_data,
7067
Field **ptabfield,
7168
uint sortlength, uint *plength);
@@ -932,22 +929,35 @@ static inline void store_length(uchar *to, uint length, uint pack_length)
932929
}
933930

934931

932+
#ifdef WORDS_BIGENDIAN
933+
const bool Is_big_endian= true;
934+
#else
935+
const bool Is_big_endian= false;
936+
#endif
937+
void copy_native_longlong(uchar *to, int to_length,
938+
longlong val, bool is_unsigned)
939+
{
940+
copy_integer<Is_big_endian>(to, to_length,
941+
static_cast<uchar*>(static_cast<void*>(&val)),
942+
sizeof(longlong),
943+
is_unsigned);
944+
}
945+
946+
935947
/** Make a sort-key from record. */
936948

937-
static void make_sortkey(register Sort_param *param,
938-
register uchar *to, uchar *ref_pos)
949+
void make_sortkey(Sort_param *param, uchar *to, uchar *ref_pos)
939950
{
940-
reg3 Field *field;
941-
reg1 SORT_FIELD *sort_field;
942-
reg5 uint length;
951+
SORT_FIELD *sort_field;
943952

944-
for (sort_field=param->local_sortorder ;
953+
for (sort_field= param->local_sortorder ;
945954
sort_field != param->end ;
946955
sort_field++)
947956
{
948-
bool maybe_null=0;
949-
if ((field=sort_field->field))
950-
{ // Field
957+
bool maybe_null= false;
958+
if (sort_field->field)
959+
{
960+
Field *field= sort_field->field;
951961
if (field->maybe_null())
952962
{
953963
if (field->is_null())
@@ -1001,7 +1011,7 @@ static void make_sortkey(register Sort_param *param,
10011011
}
10021012
break;
10031013
}
1004-
length= res->length();
1014+
uint length= res->length();
10051015
sort_field_length= sort_field->length - sort_field->suffix_length;
10061016
diff=(int) (sort_field_length - length);
10071017
if (diff < 0)
@@ -1062,27 +1072,8 @@ static void make_sortkey(register Sort_param *param,
10621072
break;
10631073
}
10641074
}
1065-
#if SIZEOF_LONG_LONG > 4
1066-
to[7]= (uchar) value;
1067-
to[6]= (uchar) (value >> 8);
1068-
to[5]= (uchar) (value >> 16);
1069-
to[4]= (uchar) (value >> 24);
1070-
to[3]= (uchar) (value >> 32);
1071-
to[2]= (uchar) (value >> 40);
1072-
to[1]= (uchar) (value >> 48);
1073-
if (item->unsigned_flag) /* Fix sign */
1074-
to[0]= (uchar) (value >> 56);
1075-
else
1076-
to[0]= (uchar) (value >> 56) ^ 128; /* Reverse signbit */
1077-
#else
1078-
to[3]= (uchar) value;
1079-
to[2]= (uchar) (value >> 8);
1080-
to[1]= (uchar) (value >> 16);
1081-
if (item->unsigned_flag) /* Fix sign */
1082-
to[0]= (uchar) (value >> 24);
1083-
else
1084-
to[0]= (uchar) (value >> 24) ^ 128; /* Reverse signbit */
1085-
#endif
1075+
copy_native_longlong(to, sort_field->length,
1076+
value, item->unsigned_flag);
10861077
break;
10871078
}
10881079
case DECIMAL_RESULT:
@@ -1098,9 +1089,20 @@ static void make_sortkey(register Sort_param *param,
10981089
}
10991090
*to++=1;
11001091
}
1101-
my_decimal2binary(E_DEC_FATAL_ERROR, dec_val, to,
1102-
item->max_length - (item->decimals ? 1:0),
1103-
item->decimals);
1092+
if (sort_field->length < DECIMAL_MAX_FIELD_SIZE)
1093+
{
1094+
uchar buf[DECIMAL_MAX_FIELD_SIZE];
1095+
my_decimal2binary(E_DEC_FATAL_ERROR, dec_val, buf,
1096+
item->max_length - (item->decimals ? 1:0),
1097+
item->decimals);
1098+
memcpy(to, buf, sort_field->length);
1099+
}
1100+
else
1101+
{
1102+
my_decimal2binary(E_DEC_FATAL_ERROR, dec_val, to,
1103+
item->max_length - (item->decimals ? 1:0),
1104+
item->decimals);
1105+
}
11041106
break;
11051107
}
11061108
case REAL_RESULT:
@@ -1116,7 +1118,16 @@ static void make_sortkey(register Sort_param *param,
11161118
}
11171119
*to++=1;
11181120
}
1119-
change_double_for_sort(value,(uchar*) to);
1121+
if (sort_field->length < sizeof(double))
1122+
{
1123+
uchar buf[sizeof(double)];
1124+
change_double_for_sort(value, buf);
1125+
memcpy(to, buf, sort_field->length);
1126+
}
1127+
else
1128+
{
1129+
change_double_for_sort(value, (uchar*) to);
1130+
}
11201131
break;
11211132
}
11221133
case ROW_RESULT:
@@ -1130,7 +1141,7 @@ static void make_sortkey(register Sort_param *param,
11301141
{ /* Revers key */
11311142
if (maybe_null)
11321143
to[-1]= ~to[-1];
1133-
length=sort_field->length;
1144+
uint length= sort_field->length;
11341145
while (length--)
11351146
{
11361147
*to = (uchar) (~ *to);
@@ -1154,7 +1165,7 @@ static void make_sortkey(register Sort_param *param,
11541165
DBUG_ASSERT(addonf != 0);
11551166
memset(nulls, 0, addonf->offset);
11561167
to+= addonf->offset;
1157-
for ( ; (field= addonf->field) ; addonf++)
1168+
for (Field* field; (field= addonf->field) ; addonf++)
11581169
{
11591170
if (addonf->null_bit && field->is_null())
11601171
{
@@ -1791,15 +1802,14 @@ static uint suffix_length(ulong string_length)
17911802
Total length of sort buffer in bytes
17921803
*/
17931804

1794-
static uint
1805+
uint
17951806
sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length,
17961807
bool *multi_byte_charset)
17971808
{
1798-
reg2 uint length;
1809+
uint total_length= 0;
17991810
const CHARSET_INFO *cs;
1800-
*multi_byte_charset= 0;
1811+
*multi_byte_charset= false;
18011812

1802-
length=0;
18031813
for (; s_length-- ; sortorder++)
18041814
{
18051815
sortorder->need_strxnfrm= 0;
@@ -1816,7 +1826,7 @@ sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length,
18161826
sortorder->length= cs->coll->strnxfrmlen(cs, sortorder->length);
18171827
}
18181828
if (sortorder->field->maybe_null())
1819-
length++; // Place for NULL marker
1829+
total_length++; // Place for NULL marker
18201830
}
18211831
else
18221832
{
@@ -1825,7 +1835,7 @@ sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length,
18251835
sortorder->result_type= INT_RESULT;
18261836
switch (sortorder->result_type) {
18271837
case STRING_RESULT:
1828-
sortorder->length=sortorder->item->max_length;
1838+
sortorder->length= sortorder->item->max_length;
18291839
set_if_smaller(sortorder->length, thd->variables.max_sort_length);
18301840
if (use_strnxfrm((cs=sortorder->item->collation.collation)))
18311841
{
@@ -1848,6 +1858,7 @@ sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length,
18481858
#endif
18491859
break;
18501860
case DECIMAL_RESULT:
1861+
// NOTE: max length here is 65 bytes, we might need to truncate!
18511862
sortorder->length=
18521863
my_decimal_get_binary_size(sortorder->item->max_length -
18531864
(sortorder->item->decimals ? 1 : 0),
@@ -1863,14 +1874,14 @@ sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length,
18631874
break;
18641875
}
18651876
if (sortorder->item->maybe_null)
1866-
length++; // Place for NULL marker
1877+
total_length++; // Place for NULL marker
18671878
}
18681879
set_if_smaller(sortorder->length, thd->variables.max_sort_length);
1869-
length+=sortorder->length;
1880+
total_length+= sortorder->length;
18701881
}
1871-
sortorder->field= (Field*) 0; // end marker
1872-
DBUG_PRINT("info",("sort_length: %d",length));
1873-
return length;
1882+
sortorder->field= NULL; // end marker
1883+
DBUG_PRINT("info",("sort_length: %u", total_length));
1884+
return total_length;
18741885
}
18751886

18761887

‎sql/filesort.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,11 @@ ha_rows filesort(THD *thd, TABLE *table, Filesort *fsort, bool sort_positions,
6868
void filesort_free_buffers(TABLE *table, bool full);
6969
void change_double_for_sort(double nr,uchar *to);
7070

71+
class Sort_param;
72+
/// Declared here so we can unit test it.
73+
void make_sortkey(Sort_param *param, uchar *to, uchar *ref_pos);
74+
/// Declared here so we can unit test it.
75+
uint sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length,
76+
bool *multi_byte_charset);
77+
7178
#endif /* FILESORT_INCLUDED */

‎unittest/gunit/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ SET(SERVER_TESTS
267267
item
268268
item_func_now_local
269269
join_tab_sort
270+
make_sortkey
270271
my_decimal
271272
opt_range
272273
opt_trace

0 commit comments

Comments
 (0)