maria-discuss team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Reindl,

It is being discussed: https://github.com/MariaDB/server/pull/378

In a nutshell:
- some are most probably false positives from coverity
- silent file name truncation suggested by this patch doesn't look good

Regards,
Sergey

On Thu, Jul 13, 2017 at 01:38:14PM +0200, Reindl Harald wrote:
> how long does Fedora need to ship that patch which seems to apply still to
> 10.2.7 where i am asking mysqlf for years if i should include it in my own
> builds or trust upstream

> The following problems have been found by Coverity - static analysis tool.
> 
> mysql-5.5.31/plugin/semisync/semisync_master.cc:672:parameter_as_source ??? Note: This defect has an elevated risk because the source argument is a parameter of the current function. 
> 
> mysql-5.5.31/plugin/semisync/semisync_master.cc:661:parameter_as_source ??? Note: This defect has an elevated risk because the source argument is a parameter of the current function. 
> 
> mysql-5.5.31/plugin/semisync/semisync_master.cc:555:parameter_as_source ??? Note: This defect has an elevated risk because the source argument is a parameter of the current function.
> 
> diff -up mariadb-10.0.21/plugin/semisync/semisync_master.cc.orig mariadb-10.0.21/plugin/semisync/semisync_master.cc
> --- mariadb-10.0.21/plugin/semisync/semisync_master.cc.orig	2015-08-05 20:11:31.000000000 +0200
> +++ mariadb-10.0.21/plugin/semisync/semisync_master.cc	2015-08-10 17:22:31.700604043 +0200
> @@ -553,7 +553,8 @@ int ReplSemiSyncMaster::reportReplyBinlo
>  
>    if (need_copy_send_pos)
>    {
> -    strcpy(reply_file_name_, log_file_name);
> +    strncpy(reply_file_name_, log_file_name, sizeof(reply_file_name_)-1);
> +    reply_file_name_[sizeof(reply_file_name_)-1] = '\0';
>      reply_file_pos_ = log_file_pos;
>      reply_file_name_inited_ = true;
>  
> @@ -661,7 +662,8 @@ int ReplSemiSyncMaster::commitTrx(const
>          if (cmp <= 0)
>  	{
>            /* This thd has a lower position, let's update the minimum info. */
> -          strcpy(wait_file_name_, trx_wait_binlog_name);
> +          strncpy(wait_file_name_, trx_wait_binlog_name, sizeof(wait_file_name_)-1);
> +          wait_file_name_[sizeof(wait_file_name_)-1] = '\0';
>            wait_file_pos_ = trx_wait_binlog_pos;
>  
>            rpl_semi_sync_master_wait_pos_backtraverse++;
> @@ -672,7 +674,8 @@ int ReplSemiSyncMaster::commitTrx(const
>        }
>        else
>        {
> -        strcpy(wait_file_name_, trx_wait_binlog_name);
> +        strncpy(wait_file_name_, trx_wait_binlog_name, sizeof(wait_file_name_)-1);
> +        wait_file_name_[sizeof(wait_file_name_)-1] = '\0';
>          wait_file_pos_ = trx_wait_binlog_pos;
>          wait_file_name_inited_ = true;
>  
> 
> mysql-5.5.31/sql/rpl_handler.cc:306:fixed_size_dest ??? You might overrun the 512 byte fixed-size string "log_info->log_file" by copying "log_file + dirname_length(log_file)" without checking the length. diff -up mysql-5.5.31/sql/rpl_handler.cc.covscan-stroverflow mysql-5.5.31/sql/rpl_handler.cc
> 
> diff -up mariadb-10.0.21/sql/rpl_handler.cc.orig mariadb-10.0.21/sql/rpl_handler.cc
> --- mariadb-10.0.21/sql/rpl_handler.cc.orig	2015-08-05 20:11:31.000000000 +0200
> +++ mariadb-10.0.21/sql/rpl_handler.cc	2015-08-10 17:28:57.093803582 +0200
> @@ -261,7 +261,8 @@ int Binlog_storage_delegate::after_flush
>      thd->semisync_info= log_info;
>    }
>  
> -  strcpy(log_info->log_file, log_file+dirname_length(log_file));
> +  strncpy(log_info->log_file, log_file+dirname_length(log_file), sizeof(log_info->log_file)-1);
> +  log_info->log_file[sizeof(log_info->log_file)-1] = '\0';
>    log_info->log_pos = log_pos;
>    
>    FOREACH_OBSERVER(ret, after_flush, false,

> _______________________________________________
> Mailing list: https://launchpad.net/~maria-discuss
> Post to     : maria-discuss@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-discuss
> More help   : https://help.launchpad.net/ListHelp