[MDEV-27480] Functions don't validate arguments properly and cast/truncate values without warnings or errors Created: 2022-01-12  Updated: 2023-11-28

Status: Stalled
Project: MariaDB Server
Component/s: Server
Affects Version/s: 10.2, 10.3, 10.4, 10.5, 10.6, 10.7
Fix Version/s: 10.11

Type: Bug Priority: Major
Reporter: Elena Stepanova Assignee: Elena Stepanova
Resolution: Unresolved Votes: 1
Labels: None

Issue Links:
Relates
relates to MDEV-27208 Implement 2-ary CRC32() and the CRC32... Closed
relates to MDEV-28140 Unexpected error when UPDATE a NULL Confirmed

 Description   

Since CRC32(par, expr) and CRC32C(par, expr) presume the par part to be an already calculated CRC32 value, it has to be a 32-bit unsigned integer. However, they don't check it to be in the range, they just either truncate or roll over all other numeric values silently, without warnings or errors:

preview-10.8-MDEV-27265-misc 3d04e67d

 
MariaDB [(none)]> select crc32(4294967296,'');
 
+----------------------+
 
| crc32(4294967296,'') |
 
+----------------------+
 
|                    0 |
 
+----------------------+
 
1 row in set (0.001 sec)


MariaDB [(none)]> select crc32(1e100,'');
 
+-----------------+
 
| crc32(1e100,'') |
 
+-----------------+
 
|      4294967295 |
 
+-----------------+
 
1 row in set (0.001 sec)


MariaDB [(none)]> select crc32(10.11,'');
 
+-----------------+
 
| crc32(10.11,'') |
 
+-----------------+
 
|              10 |
 
+-----------------+
 
1 row in set (0.001 sec)


MariaDB [(none)]> select crc32(-1,'');
 
+--------------+
 
| crc32(-1,'') |
 
+--------------+
 
|   4294967295 |
 
+--------------+
 
1 row in set (0.001 sec)

It does however validate it to be numeric, so the behavior is at least inconsistent.

MariaDB [(none)]> select crc32('','');
 
+--------------+
 
| crc32('','') |
 
+--------------+
 
|            0 |
 
+--------------+
 
1 row in set, 1 warning (0.001 sec)
 
 
 
MariaDB [(none)]> show warnings;
 
+---------+------+---------------------------------------+
 
| Level   | Code | Message                               |
 
+---------+------+---------------------------------------+
 
| Warning | 1292 | Truncated incorrect INTEGER value: '' |
 
+---------+------+---------------------------------------+
 
1 row in set (0.000 sec)



 Comments   
Comment by Elena Stepanova [ 2022-01-12 ]

Actually there is some validation, but it's even more confusing – what's converted to what and why

MariaDB [(none)]> select crc32(429496729656755555555555555555555555555555555555555555555555555555555555555555555555555,'a') as x;
 
+------------+
 
| x          |
 
+------------+
 
| 3310005809 |
 
+------------+
 
1 row in set, 2 warnings (0.001 sec)
 
 
 
MariaDB [(none)]> show warnings;
 
+---------+------+--------------------------------------------------------------------------------------------------------------------------+
 
| Level   | Code | Message                                                                                                                  |
 
+---------+------+--------------------------------------------------------------------------------------------------------------------------+
 
| Warning | 1916 | Got overflow when converting '' to DECIMAL. Value truncated                                                              |
 
| Warning | 1916 | Got overflow when converting '99999999999999999999999999999999999999999999999999999999999999999' to INT. Value truncated |
 
+---------+------+--------------------------------------------------------------------------------------------------------------------------+
 
2 rows in set (0.000 sec)

Comment by Marko Mäkelä [ 2022-01-14 ]

I added the following test case:

SELECT
 
crc32(429496729656755555555555555555555555555555555555555555555555555555555555555555555555555,'') a,
 
crc32c(429496729656755555555555555555555555555555555555555555555555555555555555555555555555555,'') b,
 
crc32(-1, '') c, crc32c(-1, '') d;

Appending an empty string will not change the checksum. Only a warning is being added; the argument was already being truncated to its least significant 32 bits.

Comment by Marko Mäkelä [ 2022-01-14 ]

For the record, also the first warning with strange empty string for the following is being issued already before the parser has resolved the crc32() function:

select crc32(29496729656755555555555555555555555555555555555555555555555555555555555555555555555555,'') as x;


#0  push_warning_printf (thd=0x7f922c000d48, 
    level=level@entry=Sql_state_errno_level::WARN_LEVEL_WARN, 
    code=code@entry=1916, 
    format=0x55c014f7afb0 "Got overflow when converting '%-.128s' to %-.32s. Value truncated") at /mariadb/10.8m/sql/sql_error.cc:752
 
#1  0x000055c014739517 in decimal_operation_results (result=result@entry=2, 
    value=value@entry=0x55c014fbdc45 "", 
    type=type@entry=0x55c01509b70d "DECIMAL")
 
    at /mariadb/10.8m/sql/my_decimal.cc:57
 
#2  0x000055c014739b7a in check_result (result=2, mask=30)
 
    at /mariadb/10.8m/sql/my_decimal.h:87
 
#3  check_result_and_overflow (val=0x7f922c016ff0, result=2, mask=30)
 
    at /mariadb/10.8m/sql/my_decimal.h:277
 
#4  str2my_decimal (mask=mask@entry=30, 
    from=from@entry=0x7f922c016f28 "294967296567", '5' <repeats 74 times>, 
    length=length@entry=86, 
    charset=charset@entry=0x55c015710900 <my_charset_latin1>, 
    decimal_value=decimal_value@entry=0x7f922c016ff0, 
    end_ptr=end_ptr@entry=0x7f9242f66448)
 
    at /mariadb/10.8m/sql/my_decimal.cc:260
 
#5  0x000055c0145984df in str2my_decimal (decimal_value=0x7f922c016ff0, 
    charset=0x55c015710900 <my_charset_latin1>, length=86, 
    "294967296567", '5' <repeats 74 times>, mask=30) at /mariadb/10.8m/sql/my_decimal.h:423
 
#6  Item_decimal::Item_decimal (this=this@entry=0x7f922c016f80, thd=thd@entry=0x7f922c000d48, str_arg=0x7f922c016f28 "294967296567", '5' <repeats 74 times>, length=86, 
    charset=charset@entry=0x55c015710900 <my_charset_latin1>) at /mariadb/10.8m/sql/item.cc:3759
 
#7  0x000055c0144f5424 in MYSQLparse (thd=thd@entry=0x7f922c000d48) at /mariadb/10.8m/sql/sql_yacc.yy:14875

Comment by Marko Mäkelä [ 2022-01-14 ]

The Item::val_int() function interface returns the result in longlong and Item::null_value and Item::unsigned_flag. This means that conversions will necessarily occur in two steps: first, to 64-bit signed or unsigned integer, and then (within the 2-ary CRC32() and CRC32C() functions) to 32-bit unsigned integer.

Functions that expect an integer argument normally invoke Item::val_int(). For arguments whose type is a string, there is a wrapper that converts them to integers. That wrapper would issue a warning for invalid strings, such as '' (0) or '24/7' (24). Similarly, floating-point values will be converted by an Item::val_int() wrapper that would apparently return the integer part of the value. I consider those conversions to be outside the scope of the implementation of any function whose arguments are integers. Those affect many other functions, such as substr('',429496729656755555555555555555555555555555555555555555555555555555555555555555555555555).

I fixed those cases that clearly are within the control of the function:

select crc32(4294967296,'');
 
crc32(4294967296,'')
 
0
 
Warnings:
 
Warning	1917	Truncated value '4294967296' when converting to INT UNSIGNED
 
select crc32(1e100,'');
 
crc32(1e100,'')
 
4294967295
 
Warnings:
 
Warning	1917	Truncated value '9223372036854775807' when converting to INT UNSIGNED
 
select crc32(10.11,'');
 
crc32(10.11,'')
 
10
 
select crc32(-1,'');
 
crc32(-1,'')
 
4294967295
 
Warnings:
 
Warning	1917	Truncated value '-1' when converting to INT UNSIGNED
 
select crc32('24/7','');
 
crc32('24/7','')
 
24
 
Warnings:
 
Warning	1292	Truncated incorrect INTEGER value: '24/7'

Comment by Elena Stepanova [ 2022-01-21 ]

Re-opening as It wasn't fixed, it was decided that since it's a general legacy problem which many functions have, it should be fixed in a consistent way everywhere.

Generated at Thu Feb 08 09:53:14 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.