Fix insecure use of strcpy, strcat and sprintf in Client and SQL (continues)

[Chaloff]

…tinues)

Old style C functions strcpy(), strcat() and sprintf() are vulnerable to security issues due to lacking memory boundary checks. Replace these in the Client and SQL with safe new and/or custom functions such as snprintf() safe_strcpy() and safe_strcat(). Continue, started 75e394f6b14645b703fa8d41632a3858999b03bd.

Description

Old style C functions strcpy(), strcat() and sprintf() are vulnerable to security issues due to lacking memory boundary checks. Replace these in the Client and SQL with safe new and/or custom functions such as snprintf() safe_strcpy() and safe_strcat(). Continue, started 75e394f6b14645b703fa8d41632a3858999b03bd.

With this change FlawFinder and other static security analyzers report 287 fewer findings.

How can this PR be tested?

The FlawFinder log before changes:

$ cat flawfinder-all-vulnerabilities.html | grep "Hits ="
Hits = 14955

After the changes:

$ cat flawfinder-all-vulnerabilities.html | grep "Hits ="
Hits = 14919

The number of fixes - 36

Additionally all CI passes for this change.

Basing the PR against the correct MariaDB version

• [x] This is a bug fix and the PR is based against the earliest branch in which the bug can be reproduced

Backward compatibility

The changes are fully backward compatible.

[LinuxJedi]

@grooverdan I had a similar opinion to you on a PR that does this in other places in the codebase. @cvicentiu gave a similar point of view to @dlenski. Assuming it is technically correct, I recommend letting it through.

[knielsen]

@Chaloff , in case of overflow, snprintf() will silently truncate the result string, leaving a corrupt result. Did you carefully understand the impact of each change, making sure you fully understand the full consequences of a truncated string in each place?

Sometimes the output string is just an error message where truncation has limited consequences, other times truncation can be very serious. Silent truncation can hide bugs from dynamic analysing tools like Valgrind and ASAN used in the testsuite. So it's very important not to do these kind of modifications blindly, and be sure not to make a change without fully understanding the code around it, even though this can be very time consuming.

Hope this helps,

- Kristian.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
10 out of 20 committers have signed the CLA.

:white_check_mark: iangilfillan
:white_check_mark: janlindstrom
:white_check_mark: wet6123
:white_check_mark: FooBarrior
:white_check_mark: tvdijen
:white_check_mark: vaintroub
:white_check_mark: LinuxJedi
:white_check_mark: bnestere
:white_check_mark: dbart
:white_check_mark: Chaloff
:x: vuvova
:x: sanja-byelkin
:x: abarkov
:x: sysprg
:x: Thirunarayanan
:x: dr-m
:x: mariadb-YuchenPei
:x: knielsen
:x: montywi
:x: dmitryshulga
_{You have signed the CLA already but the status is still pending? Let us recheck it.}