Skip to content

Commit

Permalink
MDEV-34226 On startup: UBSAN: applying zero offset to null pointer in…
Browse files Browse the repository at this point in the history
… my_copy_fix_mb from strings/ctype-mb.c and other locations

nullptr+0 is an UB (undefined behavior).

- Fixing my_string_metadata_get_mb() to handle {nullptr,0} without UB.
- Fixing THD::copy_with_error() to disallow {nullptr,0} by DBUG_ASSERT().
- Fixing parse_client_handshake_packet() to call THD::copy_with_error()
  with an empty string {"",0} instead of NULL string {nullptr,0}.
  • Loading branch information
abarkov committed May 27, 2024
1 parent 7925326 commit 4a158ec
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 2 deletions.
9 changes: 9 additions & 0 deletions mysql-test/main/connect-no-db.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#
# MDEV-34226 On startup: UBSAN: applying zero offset to null pointer in my_copy_fix_mb from strings/ctype-mb.c and other locations
#
connect con1,localhost,root,,"*NO-ONE*";
SELECT database();
database()
NULL
disconnect con1;
connection default;
10 changes: 10 additions & 0 deletions mysql-test/main/connect-no-db.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
--echo #
--echo # MDEV-34226 On startup: UBSAN: applying zero offset to null pointer in my_copy_fix_mb from strings/ctype-mb.c and other locations
--echo #

# Connect without a database

connect (con1,localhost,root,,"*NO-ONE*");
SELECT database();
disconnect con1;
connection default;
4 changes: 3 additions & 1 deletion sql/sql_acl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13799,9 +13799,11 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
Since 4.1 all database names are stored in utf8
The cast is ok as copy_with_error will create a new area for db
*/
DBUG_ASSERT(db || !db_len);
// Don't pass db==nullptr to avoid UB nullptr+0 inside copy_with_error()
if (unlikely(thd->copy_with_error(system_charset_info,
(LEX_STRING*) &mpvio->db,
thd->charset(), db, db_len)))
thd->charset(), db ? db : "", db_len)))
return packet_error;

user_len= copy_and_convert(user_buff, sizeof(user_buff) - 1,
Expand Down
2 changes: 2 additions & 0 deletions sql/sql_class.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2522,6 +2522,8 @@ bool THD::copy_with_error(CHARSET_INFO *dstcs, LEX_STRING *dst,
CHARSET_INFO *srccs,
const char *src, size_t src_length)
{
// Don't allow NULL to avoid UB in the called functions: nullptr+0
DBUG_ASSERT(src);
String_copier_with_error status;
return copy_fix(dstcs, dst, srccs, src, src_length, &status) ||
status.check_errors(srccs, src, src_length);
Expand Down
2 changes: 1 addition & 1 deletion strings/ctype.c
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ static void
my_string_metadata_get_mb(MY_STRING_METADATA *metadata,
CHARSET_INFO *cs, const char *str, ulong length)
{
const char *strend= str + length;
const char *strend= str ? str + length : NULL; // Avoid UB nullptr+0
for (my_string_metadata_init(metadata) ;
str < strend;
metadata->char_length++)
Expand Down

0 comments on commit 4a158ec

Please sign in to comment.