Bug #101767 Allocated by `new[]` but deallocated by `delete`
Submitted: 26 Nov 2020 5:52 Modified: 18 Feb 18:59
Reporter: Ella Ma Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Security: Encryption Severity:S3 (Non-critical)
Version:8.0 OS:Any
Assigned to: CPU Architecture:Any
Tags: bad deallocation

[26 Nov 2020 5:52] Ella Ma
Description:
This bug is reported by my static analyzer on smart pointers.

In function Buffered_file_io::load_file_into_buffer in file plugin/keyring/buffered_file_io.cc on line 173, a unique_ptr is declared with type `uchar` but initialized with memory allocated from `new uchar[input_buffer_size]`. This will lead to deallocating the memory with `delete` operator rather than `delete[]` operator.

It seems that such a problem will not trigger a crash on my Linux x86_64 system, but it is undefined behavior.

How to repeat:
In file plugin/keyring/buffered_file_io.cc:
168   // we'll read if there's something to read                                
169   if (likely(input_buffer_size > 0)) {
170     // do we have file format mismatch
171     if (file_arch != native_arch) { 
172       // load data to temp buffer
173       std::unique_ptr<uchar> tmp(new uchar[input_buffer_size]);

    A unique_ptr is declared with type argument of `uchar` but initialized with the memory allocated from `new[]` operator.

174       if (file_io.read(file, tmp.get(), input_buffer_size, MYF(MY_WME)) !=
175           input_buffer_size)                                                
176         return true;                                                        
177 
178       // convert data to appropriate format, leave on error                 
179       std::string converted;
180       if (Converter::convert_data(reinterpret_cast<const char *>(tmp.get()),
181                                   input_buffer_size, file_arch, native_arch,
182                                   converted))
183         return true;
184     
185       // prepare buffer size and store converted data
186       buffer->reserve(converted.length());                                  
187       memcpy(buffer->data, converted.c_str(), converted.length());          

   This problem is triggered by the destructor of std::unique_ptr<uchar>.

188     } else {                                                                
    ...

Suggested fix:
Replace `std::unique_ptr<uchar>` with `std::unique_ptr<uchar[]>`.

Or create the buffer with std::make_unique function like:
```
auto tmp = std::make_unique<uchar[]>(input_buffer_size);
```
[30 Nov 2020 15:06] MySQL Verification Team
Hi Mr. Ma,

Thank you for your bug report.

We fully agree with your analysis. unique_ptr<> that is allocated by new[] should be deallocated by delete[].

Verified as reported.
[30 Nov 2020 15:11] MySQL Verification Team
Setting correct category.
[18 Feb 18:59] Paul DuBois
Posted by developer:
 
Fixed in 8.0.24.

Code cleanup. No changelog entry required.
[19 Feb 13:16] MySQL Verification Team
Thank you, Paul.