Bug #20769 Dangling pointer error in test case ctype_recoding

Description:
There is a memory bug in the server code which manifests itself in the ctype_recoding test case.

The problem is detected by Valgrind:

VALGRIND: 'Invalid write of size 8'
    COUNT: 1
    FUNCTION: THD::rollback_item_tree_changes()    FILES:    master.err
    TESTS:    ctype_recoding
    STACK: at 0x59AE96: THD::rollback_item_tree_changes() (sql_class.cc:842)
             by 0x61FD71: Prepared_statement::cleanup_stmt() (sql_prepare.cc:2701)
             by 0x620025: Prepared_statement::prepare(char const*, unsigned) (sql_prepare.cc:2812)
             by 0x61E038: mysql_stmt_prepare(THD*, char const*, unsigned) (sql_prepare.cc:1869)
             by 0x5C1EEF: dispatch_command(enum_server_command, THD*, char*, unsigned) (sql_parse.cc:1728)
             by 0x5C06BC: do_command(THD*) (sql_parse.cc:1538)
             by 0x5BFBDC: handle_one_connection (sql_parse.cc:1175)
             by 0x4D51C63: start_thread (in /lib64/tls/libpthread-0.60.so)
             by 0x540D242: clone (in /lib64/tls/libc-2.3.2.so)
           Address 0x40AA7F0 is not stack'd, malloc'd or (recently) free'd

This bug is similar to the problem solved in BUG#19633/BUG#18037.

The problem is that a stack location is passed to THD::nocheck_register_item_tree_change(), which stores the location in a list. Then at function return the stack location stored in the list becomes invalid, causing random stack memory to be overwritten later in THD::rollback_item_tree_changes().

The real bug happens in this stack trace:

#0  THD::nocheck_register_item_tree_change at sql_class.cc:819
#1  0x0818d522 in THD::change_item_tree at sql_class.h:1536
#2  0x0817bf3c in agg_item_charsets at item.cc:1447
#3  0x081ac3ee in Item_func::agg_arg_charsets at item_func.h:183
#4  0x081c35da in Item_func_insert::fix_length_and_dec at item_strfunc.cc:990

The code in Item_func_insert::fix_length_and_dec() looks like this:

  Item *cargs[2];

  cargs[0]= args[0];
  cargs[1]= args[3];
  if (agg_arg_charsets(collation, cargs, 2, MY_COLL_ALLOW_CONV))
    return;
  args[0]= cargs[0];
  args[3]= cargs[1];

The pointer to the automatic variable cargs is passed to THD::change_item_tree() (via agg_arg_charsets()), and is rendered invalid when Item_func_insert::fix_length_and_dec() returns.

There are also two other places in item_strfunc.cc (Item_func_rpad::fix_length_and_dec() and Item_func_lpad::fix_length_and_dec()) with identical code that are likely to suffer from the same problem.

This is a serious problem, as potentially a stack overwriting could result, at the worst a remote code exeution exploit.

How to repeat:
perl mysql-test-run.pl --valgrind-all --ps-protocol ctype_recoding

It can also be reproduced with the following minimal test case:

set names koi8r;
create table t1 (c1 char(10) character set cp1251);
insert into t1 values ('ß');
select insert(c1,1,2,'ö') from t1;
drop table t1;

Suggested fix:
I think we need to pass the original Item ** locations (&args[0] and &args[3]) to THD::change_item_tree(), so that the real locations can be restored at the end of the statement, not invalidated stack locations.

The agg_arg_charsets() function must be somehow extended to allow this.

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

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

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

Pushed to mysql-5.0-release and merged into mysql-5.0.

Fixed in 5.0.23.

For documentation: This problem was a dangling stack pointer being overwritten, seen in a test case. We do not know how or under what circumstances this bug would manifest itself as crashes/corruptions for users.

Noted in 5.0.23 changelog.