Old style C functions `strcpy()`, `strcat()` and `sprintf()` are vulnerable to
security issues due to lacking memory boundary checks. Replace these in the
Connect storage engine with safe new and/or custom functions such as
`snprintf()` `safe_strcpy()` and `safe_strcat()`.
With this change FlawFinder and other static security analyzers report 287
fewer findings.
All new code of the whole pull request, including one or several files that are
either new files or modified ones, are contributed under the BSD-new license. I
am contributing on behalf of my employer Amazon Web Services, Inc.
The MariaDB code base uses strcat() and strcpy() in several
places. These are known to have memory safety issues and their usage is
discouraged. Common security scanners like Flawfinder flags them. In MariaDB we
should start using modern and safer variants on these functions.
This is similar to memory issues fixes in 19af1890b5
and 9de9f105b5 but now replace use of strcat()
and strcpy() with safer options strncat() and strncpy().
However, add '\0' forcefully to make sure the result string is correct since
for these two functions it is not guaranteed what new string will be null-terminated.
Example:
size_t dest_len = sizeof(g->Message);
strncpy(g->Message, "Null json tree", dest_len); strncat(g->Message, ":",
sizeof(g->Message) - strlen(g->Message)); size_t wrote_sz = strlen(g->Message);
size_t cur_len = wrote_sz >= dest_len ? dest_len - 1 : wrote_sz;
g->Message[cur_len] = '\0';
All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the BSD-new
license. I am contributing on behalf of my employer Amazon Web Services
-- Reviewer and co-author Vicențiu Ciorbaru <vicentiu@mariadb.org>
-- Reviewer additions:
* The initial function implementation was flawed. Replaced with a simpler
and also correct version.
* Simplified code by making use of snprintf instead of chaining strcat.
* Simplified code by removing dynamic string construction in the first
place and using static strings if possible. See connect storage engine
changes.
This commit replaces sprintf(buf, ...) with
snprintf(buf, sizeof(buf), ...),
specifically in the "easy" cases where buf is allocated with a size
known at compile time.
The changes make sure we are not write outside array/string bounds which
will lead to undefined behaviour. In case the code is trying to write
outside bounds - safe version of functions simply cut the string
messages so we process this gracefully.
All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the BSD-new
license. I am contributing on behalf of my employer Amazon Web Services,
Inc.
bsonudf.cpp warnings cleanup by Daniel Black
Reviewer: Daniel Black
The easiest way to compile and test the server with UBSAN is to run:
./BUILD/compile-pentium64-ubsan
and then run mysql-test-run.
After this commit, one should be able to run this without any UBSAN
warnings. There is still a few compiler warnings that should be fixed
at some point, but these do not expose any real bugs.
The 'special' cases where we disable, suppress or circumvent UBSAN are:
- ref10 source (as here we intentionally do some shifts that UBSAN
complains about.
- x86 version of optimized int#korr() methods. UBSAN do not like unaligned
memory access of integers. Fixed by using byte_order_generic.h when
compiling with UBSAN
- We use smaller thread stack with ASAN and UBSAN, which forced me to
disable a few tests that prints the thread stack size.
- Verifying class types does not work for shared libraries. I added
suppression in mysql-test-run.pl for this case.
- Added '#ifdef WITH_UBSAN' when using integer arithmetic where it is
safe to have overflows (two cases, in item_func.cc).
Things fixed:
- Don't left shift signed values
(byte_order_generic.h, mysqltest.c, item_sum.cc and many more)
- Don't assign not non existing values to enum variables.
- Ensure that bool and enum values are properly initialized in
constructors. This was needed as UBSAN checks that these types has
correct values when one copies an object.
(gcalc_tools.h, ha_partition.cc, item_sum.cc, partition_element.h ...)
- Ensure we do not called handler functions on unallocated objects or
deleted objects.
(events.cc, sql_acl.cc).
- Fixed bugs in Item_sp::Item_sp() where we did not call constructor
on Query_arena object.
- Fixed several cast of objects to an incompatible class!
(Item.cc, Item_buff.cc, item_timefunc.cc, opt_subselect.cc, sql_acl.cc,
sql_select.cc ...)
- Ensure we do not do integer arithmetic that causes over or underflows.
This includes also ++ and -- of integers.
(Item_func.cc, Item_strfunc.cc, item_timefunc.cc, sql_base.cc ...)
- Added JSON_VALUE_UNITIALIZED to json_value_types and ensure that
value_type is initialized to this instead of to -1, which is not a valid
enum value for json_value_types.
- Ensure we do not call memcpy() when second argument could be null.
- Fixed that Item_func_str::make_empty_result() creates an empty string
instead of a null string (safer as it ensures we do not do arithmetic
on null strings).
Other things:
- Changed struct st_position to an OBJECT and added an initialization
function to it to ensure that we do not copy or use uninitialized
members. The change to a class was also motived that we used "struct
st_position" and POSITION randomly trough the code which was
confusing.
- Notably big rewrite in sql_acl.cc to avoid using deleted objects.
- Changed in sql_partition to use '^' instead of '-'. This is safe as
the operator is either 0 or 0x8000000000000000ULL.
- Added check for select_nr < INT_MAX in JOIN::build_explain() to
avoid bug when get_select() could return NULL.
- Reordered elements in POSITION for better alignment.
- Changed sql_test.cc::print_plan() to use pointers instead of objects.
- Fixed bug in find_set() where could could execute '1 << -1'.
- Added variable have_sanitizer, used by mtr. (This variable was before
only in 10.5 and up). It can now have one of two values:
ASAN or UBSAN.
- Moved ~Archive_share() from ha_archive.cc to ha_archive.h and marked
it virtual. This was an effort to get UBSAN to work with loaded storage
engines. I kept the change as the new place is better.
- Added in CONNECT engine COLBLK::SetName(), to get around a wrong cast
in tabutil.cpp.
- Added HAVE_REPLICATION around usage of rgi_slave, to get embedded
server to compile with UBSAN. (Patch from Marko).
- Added #ifdef for powerpc64 to avoid a bug in old gcc versions related
to integer arithmetic.
Changes that should not be needed but had to be done to suppress warnings
from UBSAN:
- Added static_cast<<uint16_t>> around shift to get rid of a LOT of
compiler warnings when using UBSAN.
- Had to change some '/' of 2 base integers to shift to get rid of
some compile time warnings.
Reviewed by:
- Json changes: Alexey Botchkov
- Charset changes in ctype-uca.c: Alexander Barkov
- InnoDB changes & Embedded server: Marko Mäkelä
- sql_acl.cc changes: Vicențiu Ciorbaru
- build_explain() changes: Sergey Petrunia
filamtxt.cpp: DOSFAM::RenameTempFile: Change sprintf to snprintf.
filamvct.cpp: VECFAM::RenameTempFile: Change sprintf to snprintf.
javaconn.cpp:
Add JAVAConn::GetUTFString function.
Use it instead of env->GetStringUTFChars.
Fix wrong identation.
javaconn.h: Add GetUTFString declaration.
jdbconn.cpp:
Use GetUTFString function instead of env->GetStringUTFChars.
jmgoconn.cpp:
Use GetUTFString function instead of env->GetStringUTFChars.
Fix wrong identation.
jsonudf.cpp: change 139 to BMX line 4631.
tabjmg.cpp:
Add ReleaseStringUTF.
Fix wrong identation.
tabpivot.cpp: Fix wrong identation.
tabutil.cpp: TDBPRX::GetSubTable: Change sprintf to snprintf.
modified: storage/connect/filamtxt.cpp
modified: storage/connect/filamvct.cpp
modified: storage/connect/javaconn.cpp
modified: storage/connect/javaconn.h
modified: storage/connect/jdbconn.cpp
modified: storage/connect/jmgoconn.cpp
modified: storage/connect/jsonudf.cpp
modified: storage/connect/tabjmg.cpp
modified: storage/connect/tabpivot.cpp
modified: storage/connect/tabutil.cpp
- Fix MDEV-16895 CONNECT engine's get_error_message can cause buffer
overflow and server crash with long queries
ha_connect_cc: Update version.
get_error_message: Remove charset conversion.
modified: storage/connect/ha_connect.cc
- Fix a server crash on inserting bigint to a JDBC table
JDBConn::SetUUID:
Suppress check on ctyp that causes a server crash because ctyp
can be negative and this triggers an DEBUG_ASSERT on return.
modified: storage/connect/jdbconn.cpp
- Update jdbc.result
mysql-test/connect/r/jdbc.result: Recorded to reflect a message change.
modified: storage/connect/mysql-test/connect/r/jdbc.result
Do not silence uncertain cases, or fix any bugs.
The only functional change should be that ha_federated::extra()
is not calling DBUG_PRINT to report an unhandled case for
HA_EXTRA_PREPARE_FOR_DROP.
From the previous branch:
commit eda4928ff122a0845baf5ade83b4aa29244a3a89
Author: Olivier Bertrand <bertrandop@gmail.com>
Date: Mon Mar 9 22:34:56 2015 +0100
- Add discovery to JSON tables
When columns are not defined, CONNECT analyses the json file to find column definitions.
This wors only on table that are an array of objects. Pair keys are used to generate the
column names and pair values are used for its definition. When the LEVEL option is defined
as a not null integer, the eventual JPATH is scanned up to the LEVEL value.
From the current one:
- Fix MDEV-7521 when column names are utf8 encoded (not a general multi-charset fix)
- Adds more to JSON discovery processing and UDF's
- Use PlugDup everywhere it replaces PlugSubAlloc + strcpy.
the beginning. Defining the STRING class and begining to use it (MYSQL)
2) Change the xtrace, use_tempfile and exact_info connect variables from
GLOBAL to SESSION. Remaining GLOBAL variables have been made readonly.
3) Take care of LEX_STRING variables. The .str should not be regarded as
allways being 0 terminated. This is handled by the Strz functions that
make sure to return 0 terminated strings.
Bug fix:
- When inserting in MYSQL table with special column(s) a query such as:
insert into t2 values(0,4,'new04'),(0,5,'new05');
failed saying: column id (the special column) not found in t2.
It is now accepted but must be counted in values (these 0 are ignored)
- ROWID was returning row numbers based 0. Now it is from base 1.
modified:
storage/connect/array.cpp
storage/connect/blkfil.cpp
storage/connect/colblk.cpp
storage/connect/connect.cc
storage/connect/filamap.cpp
storage/connect/filamdbf.cpp
storage/connect/filamfix.cpp
storage/connect/filamtxt.cpp
storage/connect/filamvct.cpp
storage/connect/filamzip.cpp
storage/connect/filamzip.h
storage/connect/filter.cpp
storage/connect/global.h
storage/connect/ha_connect.cc
storage/connect/ha_connect.h
storage/connect/libdoc.cpp
storage/connect/mycat.cc
storage/connect/myconn.cpp
storage/connect/odbconn.cpp
storage/connect/plgdbutl.cpp
storage/connect/plugutil.c
storage/connect/reldef.cpp
storage/connect/tabcol.cpp
storage/connect/tabdos.cpp
storage/connect/tabfix.cpp
storage/connect/tabfmt.cpp
storage/connect/table.cpp
storage/connect/tabmul.cpp
storage/connect/tabmysql.cpp
storage/connect/tabmysql.h
storage/connect/taboccur.cpp
storage/connect/tabodbc.cpp
storage/connect/tabpivot.cpp
storage/connect/tabsys.cpp
storage/connect/tabtbl.cpp
storage/connect/tabutil.cpp
storage/connect/tabvct.cpp
storage/connect/tabwmi.cpp
storage/connect/tabwmi.h
storage/connect/tabxcl.cpp
storage/connect/tabxml.cpp
storage/connect/user_connect.cc
storage/connect/valblk.cpp
storage/connect/value.cpp
storage/connect/value.h
storage/connect/xindex.cpp
storage/connect/xobject.cpp
storage/connect/xobject.h
storage/connect/xtable.h
them sorted by file position. Firstly a new value is stored in indexes
to know if they are sorted, preventing to do the sorting when it is not
needed. Secondly, almost all in now done in connect instead of being
done by the different file access method classes. This pepares the future
use of temporary files for all table types and also fix the bug that was
occuring when partially using a multi-column index because of false MRR
like call of position followed by unsorted rnd_pos no more using indexing.
modified:
storage/connect/connect.cc
storage/connect/filamap.cpp
storage/connect/filamap.h
storage/connect/filamdbf.cpp
storage/connect/filamdbf.h
storage/connect/filamfix.cpp
storage/connect/filamfix.h
storage/connect/filamtxt.cpp
storage/connect/filamtxt.h
storage/connect/filamvct.cpp
storage/connect/filamvct.h
storage/connect/tabdos.cpp
storage/connect/tabdos.h
storage/connect/tabfix.h
storage/connect/tabfmt.cpp
storage/connect/tabfmt.h
storage/connect/xindex.cpp
storage/connect/xindex.h
storage/connect/xtable.h
have been created by the last addition of new CONNECT features.
The version previous to this one is a preliminary test version and
should not be distributed.
- Handle indexed UPDATE/DELETE. Previously this was just tested and
an error message send when it could not be done. Now CONNECT can
do it in all the cases. It is done by a MRR like tchnique by making
a list of all update or delete to do, sort them, then execute them.
modified:
storage/connect/array.cpp
storage/connect/array.h
storage/connect/filamap.cpp
storage/connect/filamap.h
storage/connect/filamdbf.cpp
storage/connect/filamfix.cpp
storage/connect/filamfix.h
storage/connect/filamtxt.cpp
storage/connect/filamtxt.h
storage/connect/filamvct.cpp
storage/connect/filamvct.h
storage/connect/filamzip.cpp
storage/connect/filamzip.h
storage/connect/global.h
storage/connect/ha_connect.cc
storage/connect/ha_connect.h
- Differenciate Cardinality that returns a true or estimated table size
and GetMaxSize that return a value equal or greater than the table
row number. This fixes the errors of non matching opt files.
modified:
storage/connect/connect.cc
storage/connect/tabdos.cpp
storage/connect/tabdos.h
storage/connect/tabfix.cpp
storage/connect/table.cpp
storage/connect/tabmac.h
storage/connect/tabmysql.cpp
storage/connect/tabmysql.h
storage/connect/tabodbc.cpp
storage/connect/tabodbc.h
storage/connect/tabpivot.h
storage/connect/tabtbl.cpp
storage/connect/tabtbl.h
storage/connect/tabutil.cpp
storage/connect/tabutil.h
storage/connect/tabwmi.h
storage/connect/xtable.h
- Fix some errors and issues when making index and opt files.
Erase opt and index files for void tables.
Fix wrong calculation of Block and Last in MakeBlockValues.
Invalidate indexes before making opt file.
Fully handle blocked variable tables. Make opt file for blocked
variable tables even when they have no optimised colums.
modified:
storage/connect/tabdos.cpp
storage/connect/xindex.h
- Fix some errors making index
Return an error when the allocation is too small (should not
really occur now that GetMaxSize is sure)
Don't use XXROW index for DBF tables because of soft deleted lines.
modified:
storage/connect/xindex.cpp
- Typo
modified:
storage/connect/macutil.cpp
storage/connect/tabdos.h
storage/connect/tabsys.cpp
storage/connect/tabsys.h
1) Support of partitioning by connect. A table can be partitioned
by files, this is an enhanced MULTIPLE table. It can be also
partitioned by sub-tables like TBL and this enables table sharding.
2) Handling a CONNECT bug that causes in some cases extraneous rows
to remain in the table after an UPDATE or DELETE when the command
uses indexing (for not fixed file tables). Until a real fix is
done, CONNECT tries to ignore indexing and if it cannot do it
abort the command with an error message.
- Add tests on partitioning
added:
storage/connect/mysql-test/connect/r/part_file.result
storage/connect/mysql-test/connect/r/part_table.result
storage/connect/mysql-test/connect/t/part_file.test
storage/connect/mysql-test/connect/t/part_table.test
- Temporary fix
modified:
sql/sql_partition.cc
- Add partition support
modified:
storage/connect/ha_connect.cc
storage/connect/ha_connect.h
storage/connect/reldef.cpp
storage/connect/reldef.h
storage/connect/tabdos.cpp
- Add functions ha_connect::IsUnique and ha_connect::CheckColumnList
modified:
storage/connect/ha_connect.cc
storage/connect/ha_connect.h
- Prevent updating a partition table column that is part of
the partition function (outward tables only)
modified:
storage/connect/ha_connect.cc
- Support INSERT/UPDATE/DELETE for PROXY tables
modified:
storage/connect/tabutil.cpp
- Handle the bug on updating rows via indexing. Waiting for a real fix,
Don't use indexing when possible else raise an error and abort.
modified:
storage/connect/ha_connect.cc
- dbuserp->UseTemp set to TMP_AUTO
modified:
storage/connect/connect.cc
- Add members nox, abort and only
modified:
storage/connect/ha_connect.cc
storage/connect/ha_connect.h
- Add arguments nox and abort to CntCloseTable
modified:
storage/connect/connect.cc
storage/connect/connect.h
storage/connect/filamap.cpp
storage/connect/filamap.h
storage/connect/filamdbf.cpp
storage/connect/filamdbf.h
storage/connect/filamfix.cpp
storage/connect/filamfix.h
storage/connect/filamtxt.cpp
storage/connect/filamtxt.h
storage/connect/filamvct.cpp
storage/connect/filamvct.h
storage/connect/filamzip.cpp
storage/connect/filamzip.h
storage/connect/ha_connect.cc
- Add arguments abort to CloseTableFile and RenameTempFile
modified:
storage/connect/filamap.cpp
storage/connect/filamap.h
storage/connect/filamdbf.cpp
storage/connect/filamdbf.h
storage/connect/filamfix.cpp
storage/connect/filamfix.h
storage/connect/filamtxt.cpp
storage/connect/filamtxt.h
storage/connect/filamvct.cpp
storage/connect/filamvct.h
storage/connect/filamzip.cpp
storage/connect/filamzip.h
storage/connect/tabdos.cpp
storage/connect/tabdos.h
storage/connect/tabvct.cpp
storage/connect/xtable.h
- Fix info->records when file does not exists
modified:
storage/connect/connect.cc
- Close XML table when opened for info
modified:
storage/connect/connect.cc
- Add function VCTFAM::GetFileLength
modified:
storage/connect/filamvct.cpp
storage/connect/filamvct.h
- Column option DISTRIB -> ENUM
modified:
storage/connect/ha_connect.cc
- Options connect, query_string and partname allways available
modified:
storage/connect/ha_connect.cc
- Add function MYSQLC::GetTableSize
modified:
storage/connect/myconn.cpp
storage/connect/myconn.h
- Add new special columns (PARTNAME, FNAME, FPATH, FTYPE and FDISK)
modified:
storage/connect/colblk.cpp
storage/connect/colblk.h
storage/connect/plgdbsem.h
storage/connect/table.cpp
- Add function ExtractFromPath
modified:
storage/connect/colblk.cpp
storage/connect/plgdbsem.h
storage/connect/plgdbutl.cpp
- Enhance Cardinality for some table types
modified:
storage/connect/tabdos.cpp
storage/connect/tabmysql.cpp
storage/connect/tabmysql.h
storage/connect/tabodbc.cpp
storage/connect/tabodbc.h
storage/connect/tabsys.cpp
storage/connect/tabsys.h
storage/connect/xindex.cpp
storage/connect/xindex.h
storage/connect/xtable.h
- Add test on special column
modified:
storage/connect/tabfmt.cpp
- Add new files (added for block indexing)
modified:
storage/connect/CMakeLists.txt
