Skip to content

Commit

Permalink
MDEV-23999 Potential stack overflow in InnoDB fulltext search
Browse files Browse the repository at this point in the history
fts_query_t::nested_sub_exp: Keep track of nested
fts_ast_visit_sub_exp() calls.

fts_ast_visit_sub_exp(): Return DB_OUT_OF_MEMORY if the
maximum recursion depth is exceeded.

This is motivated by a change in MySQL 5.6.50:
mysql/mysql-server@e2a46b4
Bug #29929684 USING MANY NESTED ARGUMENTS WITH BOOLEAN FTS CAN LEAD
TO TERMINATE SERVER
  • Loading branch information
dr-m committed Oct 21, 2020
1 parent 0627c4a commit c755296
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 34 deletions.
21 changes: 21 additions & 0 deletions mysql-test/suite/innodb_fts/r/basic.result
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,27 @@ id title body
3 Optimizing MySQL In this tutorial we will show ...
4 1001 MySQL Tricks 1. Never run mysqld as root. 2. ...
5 MySQL vs. YourSQL In the following database comparison ...
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('(((((((((((((((((((((((((((((((((Security)))))))))))))))))))))))))))))))))'
IN BOOLEAN MODE);
ERROR HY000: Table handler out of memory
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('((((((((((((((((((((((((((((((((Security))))))))))))))))))))))))))))))))'
IN BOOLEAN MODE);
id title body
6 MySQL Security When configured properly, MySQL ...
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('(((((((((((((((((((((((((((((((vs))))))))))))))))))))))))))))))),(((to)))'
IN BOOLEAN MODE);
id title body
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('((((((((((((((((((((((((((((((((Security)))))))))))))))))))))))))))))))'
IN BOOLEAN MODE);
ERROR 42000: syntax error, unexpected $end
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('(((((((((((((((((((((((((((((((((Security))))))))))))))))))))))))))))))))'
IN BOOLEAN MODE);
ERROR 42000: syntax error, unexpected $end
SELECT * FROM articles WHERE MATCH (title,body)
AGAINST ('+ MySQL + (>Well < stands)' IN BOOLEAN MODE);
id title body
Expand Down
20 changes: 20 additions & 0 deletions mysql-test/suite/innodb_fts/t/basic.test
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,26 @@ SELECT * FROM articles WHERE MATCH (title,body)
SELECT * FROM articles WHERE MATCH (title,body)
AGAINST ('+ MySQL - (Well stands)' IN BOOLEAN MODE);

--error 128
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('(((((((((((((((((((((((((((((((((Security)))))))))))))))))))))))))))))))))'
IN BOOLEAN MODE);
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('((((((((((((((((((((((((((((((((Security))))))))))))))))))))))))))))))))'
IN BOOLEAN MODE);
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('(((((((((((((((((((((((((((((((vs))))))))))))))))))))))))))))))),(((to)))'
IN BOOLEAN MODE);

--error ER_PARSE_ERROR
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('((((((((((((((((((((((((((((((((Security)))))))))))))))))))))))))))))))'
IN BOOLEAN MODE);
--error ER_PARSE_ERROR
SELECT * FROM articles WHERE MATCH (title,body) AGAINST
('(((((((((((((((((((((((((((((((((Security))))))))))))))))))))))))))))))))'
IN BOOLEAN MODE);

# Test sub-expression boolean search. Find rows contain
# "MySQL" and "Well" or "MySQL" and "stands". But rank the
# doc with "Well" higher, and doc with "stands" lower.
Expand Down
19 changes: 16 additions & 3 deletions storage/innobase/fts/fts0que.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2007, 2018, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, 2019, MariaDB Corporation.
Copyright (c) 2007, 2020, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, 2020, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -146,6 +146,8 @@ struct fts_query_t {
fts_word_freq_t */

bool multi_exist; /*!< multiple FTS_EXIST oper */
byte visiting_sub_exp; /*!< count of nested
fts_ast_visit_sub_exp() */
};

/** For phrase matching, first we collect the documents and the positions
Expand Down Expand Up @@ -2836,6 +2838,8 @@ fts_query_get_token(
return(new_ptr);
}

static dberr_t fts_ast_visit_sub_exp(fts_ast_node_t*, fts_ast_callback, void*);

/*****************************************************************//**
Visit every node of the AST. */
static
Expand Down Expand Up @@ -2925,7 +2929,7 @@ Process (nested) sub-expression, create a new result set to store the
sub-expression result by processing nodes under current sub-expression
list. Merge the sub-expression result with that of parent expression list.
@return DB_SUCCESS if all well */
UNIV_INTERN
static
dberr_t
fts_ast_visit_sub_exp(
/*==================*/
Expand All @@ -2945,6 +2949,14 @@ fts_ast_visit_sub_exp(

ut_a(node->type == FTS_AST_SUBEXP_LIST);

/* To avoid stack overflow, we limit the mutual recursion
depth between fts_ast_visit(), fts_query_visitor() and
fts_ast_visit_sub_exp(). */
if (query->visiting_sub_exp++ > 31) {
query->error = DB_OUT_OF_MEMORY;
DBUG_RETURN(query->error);
}

cur_oper = query->oper;

/* Save current result set */
Expand All @@ -2967,6 +2979,7 @@ fts_ast_visit_sub_exp(
/* Reinstate parent node state */
query->multi_exist = multi_exist;
query->oper = cur_oper;
query->visiting_sub_exp--;

/* Merge the sub-expression result with the parent result set. */
subexpr_doc_ids = query->doc_ids;
Expand Down
15 changes: 1 addition & 14 deletions storage/innobase/include/fts0ast.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2007, 2018, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2016, 2018, MariaDB Corporation.
Copyright (c) 2016, 2020, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -204,19 +204,6 @@ fts_ast_visit(
operator, currently we only
ignore FTS_IGNORE operator */
MY_ATTRIBUTE((nonnull, warn_unused_result));
/*****************************************************************//**
Process (nested) sub-expression, create a new result set to store the
sub-expression result by processing nodes under current sub-expression
list. Merge the sub-expression result with that of parent expression list.
@return DB_SUCCESS if all went well */
UNIV_INTERN
dberr_t
fts_ast_visit_sub_exp(
/*==================*/
fts_ast_node_t* node, /*!< in: instance to traverse*/
fts_ast_callback visitor, /*!< in: callback */
void* arg) /*!< in: callback arg */
MY_ATTRIBUTE((nonnull, warn_unused_result));
/********************************************************************
Create a lex instance.*/
UNIV_INTERN
Expand Down
19 changes: 16 additions & 3 deletions storage/xtradb/fts/fts0que.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2007, 2018, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, 2019, MariaDB Corporation.
Copyright (c) 2007, 2020, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, 2020, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -146,6 +146,8 @@ struct fts_query_t {
fts_word_freq_t */

bool multi_exist; /*!< multiple FTS_EXIST oper */
byte visiting_sub_exp; /*!< count of nested
fts_ast_visit_sub_exp() */
};

/** For phrase matching, first we collect the documents and the positions
Expand Down Expand Up @@ -2856,6 +2858,8 @@ fts_query_get_token(
return(new_ptr);
}

static dberr_t fts_ast_visit_sub_exp(fts_ast_node_t*, fts_ast_callback, void*);

/*****************************************************************//**
Visit every node of the AST. */
static
Expand Down Expand Up @@ -2945,7 +2949,7 @@ Process (nested) sub-expression, create a new result set to store the
sub-expression result by processing nodes under current sub-expression
list. Merge the sub-expression result with that of parent expression list.
@return DB_SUCCESS if all well */
UNIV_INTERN
static
dberr_t
fts_ast_visit_sub_exp(
/*==================*/
Expand All @@ -2965,6 +2969,14 @@ fts_ast_visit_sub_exp(

ut_a(node->type == FTS_AST_SUBEXP_LIST);

/* To avoid stack overflow, we limit the mutual recursion
depth between fts_ast_visit(), fts_query_visitor() and
fts_ast_visit_sub_exp(). */
if (query->visiting_sub_exp++ > 31) {
query->error = DB_OUT_OF_MEMORY;
DBUG_RETURN(query->error);
}

cur_oper = query->oper;

/* Save current result set */
Expand All @@ -2987,6 +2999,7 @@ fts_ast_visit_sub_exp(
/* Reinstate parent node state */
query->multi_exist = multi_exist;
query->oper = cur_oper;
query->visiting_sub_exp--;

/* Merge the sub-expression result with the parent result set. */
subexpr_doc_ids = query->doc_ids;
Expand Down
15 changes: 1 addition & 14 deletions storage/xtradb/include/fts0ast.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2007, 2018, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2016, 2018, MariaDB Corporation.
Copyright (c) 2016, 2020, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -204,19 +204,6 @@ fts_ast_visit(
operator, currently we only
ignore FTS_IGNORE operator */
MY_ATTRIBUTE((nonnull, warn_unused_result));
/*****************************************************************//**
Process (nested) sub-expression, create a new result set to store the
sub-expression result by processing nodes under current sub-expression
list. Merge the sub-expression result with that of parent expression list.
@return DB_SUCCESS if all went well */
UNIV_INTERN
dberr_t
fts_ast_visit_sub_exp(
/*==================*/
fts_ast_node_t* node, /*!< in: instance to traverse*/
fts_ast_callback visitor, /*!< in: callback */
void* arg) /*!< in: callback arg */
MY_ATTRIBUTE((nonnull, warn_unused_result));
/********************************************************************
Create a lex instance.*/
UNIV_INTERN
Expand Down

0 comments on commit c755296

Please sign in to comment.