From 294b9336069684571040b7c8cfa85d042828a87f Mon Sep 17 00:00:00 2001 From: Georg Richter Date: Tue, 10 Dec 2024 08:01:37 +0100 Subject: [PATCH 1/2] CONC-708: buffer over-/underflow in ma_read_ok_packet Added a helper function ma_check_buffer_bounaaries which checks possible boffer over- or underflows when processing ok packet.^ --- libmariadb/mariadb_lib.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/libmariadb/mariadb_lib.c b/libmariadb/mariadb_lib.c index 67b9bcc0..d1d9e2f3 100644 --- a/libmariadb/mariadb_lib.c +++ b/libmariadb/mariadb_lib.c @@ -2166,14 +2166,29 @@ mysql_send_query(MYSQL* mysql, const char* query, unsigned long length) return ma_simple_command(mysql, COM_QUERY, query, length, 1,0); } +/* Helper function to detect possible buffer over- or underflow */ +inline my_bool ma_check_buffer_boundaries(MYSQL *mysql, uchar *current_pos, + ulong packet_size, size_t required) +{ + if ( (packet_size < (ulong)(current_pos - mysql->net.read_pos)) || + ((size_t)(packet_size - (current_pos - mysql->net.read_pos)) < required)) + return 1; + return 0; +} + int ma_read_ok_packet(MYSQL *mysql, uchar *pos, ulong length) { uchar *end= mysql->net.read_pos+length; size_t item_len; mysql->affected_rows= net_field_length_ll(&pos); mysql->insert_id= net_field_length_ll(&pos); + + if (ma_check_buffer_boundaries(mysql, pos, length, 2)) + goto corrupted; mysql->server_status=uint2korr(pos); pos+=2; + if (ma_check_buffer_boundaries(mysql, pos, length, 2)) + goto corrupted; mysql->warning_count=uint2korr(pos); pos+=2; if (pos > end) @@ -2182,7 +2197,7 @@ int ma_read_ok_packet(MYSQL *mysql, uchar *pos, ulong length) { if ((item_len= net_field_length(&pos))) mysql->info=(char*) pos; - if (pos + item_len > end) + if (ma_check_buffer_boundaries(mysql, pos, length, item_len)) goto corrupted; /* check if server supports session tracking */ @@ -2202,7 +2217,7 @@ int ma_read_ok_packet(MYSQL *mysql, uchar *pos, ulong length) uchar *old_pos= pos; item_len= net_field_length(&pos); /* length for all items */ - if (pos + item_len > end) + if (ma_check_buffer_boundaries(mysql, pos, length, item_len)) goto corrupted; end= pos + item_len; @@ -2233,7 +2248,7 @@ int ma_read_ok_packet(MYSQL *mysql, uchar *pos, ulong length) net_field_length(&pos); } plen= net_field_length(&pos); - if (pos + plen > end) + if (ma_check_buffer_boundaries(mysql, pos, length, plen)) goto corrupted; if (!(session_item= ma_multi_malloc(0, &session_item, sizeof(LIST), @@ -2263,7 +2278,7 @@ int ma_read_ok_packet(MYSQL *mysql, uchar *pos, ulong length) if (!strncmp(str->str, "character_set_client", str->length)) set_charset= 1; plen= net_field_length(&pos); - if (pos + plen > end) + if (ma_check_buffer_boundaries(mysql, pos, length, plen)) goto corrupted; if (!(session_item= ma_multi_malloc(0, &session_item, sizeof(LIST), @@ -2292,7 +2307,7 @@ int ma_read_ok_packet(MYSQL *mysql, uchar *pos, ulong length) default: /* not supported yet */ plen= net_field_length(&pos); - if (pos + plen > end) + if (ma_check_buffer_boundaries(mysql, pos, length, plen)) goto corrupted; pos+= plen; break; From 6bf9557d00c656802cca08e14ed0953fa91b70f8 Mon Sep 17 00:00:00 2001 From: Georg Richter Date: Thu, 12 Dec 2024 10:40:30 +0100 Subject: [PATCH 2/2] CONC-709: Fix crash when sending NULL_LENGTH in field description Instead of checking the lengths given by two offsets, we have to check if NULL_LENGTH was sent before (and return an error). --- libmariadb/mariadb_lib.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/libmariadb/mariadb_lib.c b/libmariadb/mariadb_lib.c index d1d9e2f3..ebe03a8d 100644 --- a/libmariadb/mariadb_lib.c +++ b/libmariadb/mariadb_lib.c @@ -153,6 +153,16 @@ static int cli_report_progress(MYSQL *mysql, uchar *packet, uint length); extern int mysql_client_plugin_init(); extern void mysql_client_plugin_deinit(); +/* Helper function to detect possible buffer over- or underflow */ +my_bool ma_check_buffer_boundaries(MYSQL *mysql, uchar *current_pos, + ulong packet_size, size_t required) +{ + if ( (packet_size < (ulong)(current_pos - mysql->net.read_pos)) || + ((size_t)(packet_size - (current_pos - mysql->net.read_pos)) < required)) + return 1; + return 0; +} + /* net_get_error */ void net_get_error(char *buf, size_t buf_len, char *error, size_t error_len, @@ -890,10 +900,13 @@ unpack_fields(const MYSQL *mysql, for (i=0; i < field_count; i++) { - uint length= (uint)(row->data[i+1] - row->data[i] - 1); - if (!row->data[i] || row->data[i][length]) + uint length; + + if (!row->data[i]) goto error; + length= (uint)(row->data[i+1] - row->data[i] - 1); + *(char **)(((char *)field) + rset_field_offsets[i*2])= ma_strdup_root(alloc, (char *)row->data[i]); *(unsigned int *)(((char *)field) + rset_field_offsets[i*2+1])= length; @@ -2166,16 +2179,6 @@ mysql_send_query(MYSQL* mysql, const char* query, unsigned long length) return ma_simple_command(mysql, COM_QUERY, query, length, 1,0); } -/* Helper function to detect possible buffer over- or underflow */ -inline my_bool ma_check_buffer_boundaries(MYSQL *mysql, uchar *current_pos, - ulong packet_size, size_t required) -{ - if ( (packet_size < (ulong)(current_pos - mysql->net.read_pos)) || - ((size_t)(packet_size - (current_pos - mysql->net.read_pos)) < required)) - return 1; - return 0; -} - int ma_read_ok_packet(MYSQL *mysql, uchar *pos, ulong length) { uchar *end= mysql->net.read_pos+length;